Skip to content

Commit 79092b0

Browse files
zjjhotzjjhot
committed
Merge remote-tracking branch 'giteaofficial/main'
* giteaofficial/main: Fix duplicate entry error when add team member (go-gitea#19702) [Refactor] convert team(s) to apiTeam(s) (go-gitea#13745)
2 parents 62f9d4e + 77e9346 commit 79092b0

File tree

11 files changed

+122
-98
lines changed

11 files changed

+122
-98
lines changed

‎integrations/api_team_test.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func TestAPITeam(t *testing.T) {
6969
resp = session.MakeRequest(t, req, http.StatusCreated)
7070
apiTeam = api.Team{}
7171
DecodeJSON(t, resp, &apiTeam)
72-
checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
72+
checkTeamResponse(t, "CreateTeam1", &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
7373
teamToCreate.Permission, teamToCreate.Units, nil)
7474
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
7575
teamToCreate.Permission, teamToCreate.Units, nil)
@@ -90,7 +90,7 @@ func TestAPITeam(t *testing.T) {
9090
resp = session.MakeRequest(t, req, http.StatusOK)
9191
apiTeam = api.Team{}
9292
DecodeJSON(t, resp, &apiTeam)
93-
checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
93+
checkTeamResponse(t, "EditTeam1", &apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
9494
teamToEdit.Permission, unit.AllUnitKeyNames(), nil)
9595
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
9696
teamToEdit.Permission, unit.AllUnitKeyNames(), nil)
@@ -102,7 +102,7 @@ func TestAPITeam(t *testing.T) {
102102
resp = session.MakeRequest(t, req, http.StatusOK)
103103
apiTeam = api.Team{}
104104
DecodeJSON(t, resp, &apiTeam)
105-
checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
105+
checkTeamResponse(t, "EditTeam1_DescOnly", &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
106106
teamToEdit.Permission, unit.AllUnitKeyNames(), nil)
107107
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
108108
teamToEdit.Permission, unit.AllUnitKeyNames(), nil)
@@ -114,7 +114,7 @@ func TestAPITeam(t *testing.T) {
114114
resp = session.MakeRequest(t, req, http.StatusOK)
115115
apiTeam = api.Team{}
116116
DecodeJSON(t, resp, &apiTeam)
117-
checkTeamResponse(t, &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories,
117+
checkTeamResponse(t, "ReadTeam1", &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories,
118118
teamRead.AccessMode.String(), teamRead.GetUnitNames(), teamRead.GetUnitsMap())
119119

120120
// Delete team.
@@ -135,7 +135,7 @@ func TestAPITeam(t *testing.T) {
135135
resp = session.MakeRequest(t, req, http.StatusCreated)
136136
apiTeam = api.Team{}
137137
DecodeJSON(t, resp, &apiTeam)
138-
checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
138+
checkTeamResponse(t, "CreateTeam2", &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
139139
"read", nil, teamToCreate.UnitsMap)
140140
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
141141
"read", nil, teamToCreate.UnitsMap)
@@ -156,7 +156,7 @@ func TestAPITeam(t *testing.T) {
156156
resp = session.MakeRequest(t, req, http.StatusOK)
157157
apiTeam = api.Team{}
158158
DecodeJSON(t, resp, &apiTeam)
159-
checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
159+
checkTeamResponse(t, "EditTeam2", &apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
160160
"read", nil, teamToEdit.UnitsMap)
161161
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories,
162162
"read", nil, teamToEdit.UnitsMap)
@@ -168,7 +168,7 @@ func TestAPITeam(t *testing.T) {
168168
resp = session.MakeRequest(t, req, http.StatusOK)
169169
apiTeam = api.Team{}
170170
DecodeJSON(t, resp, &apiTeam)
171-
checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
171+
checkTeamResponse(t, "EditTeam2_DescOnly", &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
172172
"read", nil, teamToEdit.UnitsMap)
173173
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
174174
"read", nil, teamToEdit.UnitsMap)
@@ -180,7 +180,7 @@ func TestAPITeam(t *testing.T) {
180180
apiTeam = api.Team{}
181181
DecodeJSON(t, resp, &apiTeam)
182182
assert.NoError(t, teamRead.GetUnits())
183-
checkTeamResponse(t, &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories,
183+
checkTeamResponse(t, "ReadTeam2", &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories,
184184
teamRead.AccessMode.String(), teamRead.GetUnitNames(), teamRead.GetUnitsMap())
185185

186186
// Delete team.
@@ -189,8 +189,8 @@ func TestAPITeam(t *testing.T) {
189189
unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID})
190190
}
191191

192-
func checkTeamResponse(t *testing.T, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission string, units []string, unitsMap map[string]string) {
193-
t.Run(name+description, func(t *testing.T) {
192+
func checkTeamResponse(t *testing.T, testName string, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission string, units []string, unitsMap map[string]string) {
193+
t.Run(testName, func(t *testing.T) {
194194
assert.Equal(t, name, apiTeam.Name, "name")
195195
assert.Equal(t, description, apiTeam.Description, "description")
196196
assert.Equal(t, includesAllRepositories, apiTeam.IncludesAllRepositories, "includesAllRepositories")
@@ -209,7 +209,9 @@ func checkTeamResponse(t *testing.T, apiTeam *api.Team, name, description string
209209
func checkTeamBean(t *testing.T, id int64, name, description string, includesAllRepositories bool, permission string, units []string, unitsMap map[string]string) {
210210
team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: id}).(*organization.Team)
211211
assert.NoError(t, team.GetUnits(), "GetUnits")
212-
checkTeamResponse(t, convert.ToTeam(team), name, description, includesAllRepositories, permission, units, unitsMap)
212+
apiTeam, err := convert.ToTeam(team)
213+
assert.NoError(t, err)
214+
checkTeamResponse(t, fmt.Sprintf("checkTeamBean/%s_%s", name, description), apiTeam, name, description, includesAllRepositories, permission, units, unitsMap)
213215
}
214216

215217
type TeamSearchResults struct {

‎integrations/org_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func TestOrgRestrictedUser(t *testing.T) {
157157

158158
resp := adminSession.MakeRequest(t, req, http.StatusCreated)
159159
DecodeJSON(t, resp, &apiTeam)
160-
checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
160+
checkTeamResponse(t, "CreateTeam_codereader", &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
161161
teamToCreate.Permission, teamToCreate.Units, nil)
162162
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
163163
teamToCreate.Permission, teamToCreate.Units, nil)

‎models/org_team.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,12 @@ func AddTeamMember(team *organization.Team, userID int64) error {
496496
}
497497
defer committer.Close()
498498

499+
// check in transaction
500+
isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
501+
if err != nil || isAlreadyMember {
502+
return err
503+
}
504+
499505
sess := db.GetEngine(ctx)
500506

501507
if err := db.Insert(ctx, &organization.TeamUser{

‎models/organization/org.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,12 @@ func AddOrgUser(orgID, uid int64) error {
594594
}
595595
defer committer.Close()
596596

597+
// check in transaction
598+
isAlreadyMember, err = IsOrganizationMember(ctx, orgID, uid)
599+
if err != nil || isAlreadyMember {
600+
return err
601+
}
602+
597603
ou := &OrgUser{
598604
UID: uid,
599605
OrgID: orgID,

‎modules/convert/convert.go

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -304,22 +304,53 @@ func ToOrganization(org *organization.Organization) *api.Organization {
304304
}
305305
}
306306

307-
// ToTeam convert organization.Team to api.Team
308-
func ToTeam(team *organization.Team) *api.Team {
309-
if team == nil {
310-
return nil
307+
// ToTeam convert models.Team to api.Team
308+
func ToTeam(team *organization.Team, loadOrg ...bool) (*api.Team, error) {
309+
teams, err := ToTeams([]*organization.Team{team}, len(loadOrg) != 0 && loadOrg[0])
310+
if err != nil || len(teams) == 0 {
311+
return nil, err
312+
}
313+
return teams[0], nil
314+
}
315+
316+
// ToTeams convert models.Team list to api.Team list
317+
func ToTeams(teams []*organization.Team, loadOrgs bool) ([]*api.Team, error) {
318+
if len(teams) == 0 || teams[0] == nil {
319+
return nil, nil
311320
}
312321

313-
return &api.Team{
314-
ID: team.ID,
315-
Name: team.Name,
316-
Description: team.Description,
317-
IncludesAllRepositories: team.IncludesAllRepositories,
318-
CanCreateOrgRepo: team.CanCreateOrgRepo,
319-
Permission: team.AccessMode.String(),
320-
Units: team.GetUnitNames(),
321-
UnitsMap: team.GetUnitsMap(),
322+
cache := make(map[int64]*api.Organization)
323+
apiTeams := make([]*api.Team, len(teams))
324+
for i := range teams {
325+
if err := teams[i].GetUnits(); err != nil {
326+
return nil, err
327+
}
328+
329+
apiTeams[i] = &api.Team{
330+
ID: teams[i].ID,
331+
Name: teams[i].Name,
332+
Description: teams[i].Description,
333+
IncludesAllRepositories: teams[i].IncludesAllRepositories,
334+
CanCreateOrgRepo: teams[i].CanCreateOrgRepo,
335+
Permission: teams[i].AccessMode.String(),
336+
Units: teams[i].GetUnitNames(),
337+
UnitsMap: teams[i].GetUnitsMap(),
338+
}
339+
340+
if loadOrgs {
341+
apiOrg, ok := cache[teams[i].OrgID]
342+
if !ok {
343+
org, err := organization.GetOrgByID(teams[i].OrgID)
344+
if err != nil {
345+
return nil, err
346+
}
347+
apiOrg = ToOrganization(org)
348+
cache[teams[i].OrgID] = apiOrg
349+
}
350+
apiTeams[i].Organization = apiOrg
351+
}
322352
}
353+
return apiTeams, nil
323354
}
324355

325356
// ToAnnotatedTag convert git.Tag to api.AnnotatedTag

‎modules/convert/issue_comment.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func ToTimelineComment(c *models.Comment, doer *user_model.User) *api.TimelineCo
152152
comment.Assignee = ToUser(c.Assignee, nil)
153153
}
154154
if c.AssigneeTeam != nil {
155-
comment.AssigneeTeam = ToTeam(c.AssigneeTeam)
155+
comment.AssigneeTeam, _ = ToTeam(c.AssigneeTeam)
156156
}
157157

158158
if c.ResolveDoer != nil {

‎modules/convert/pull_review.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,15 @@ func ToPullReview(ctx context.Context, r *models.Review, doer *user_model.User)
2222
r.Reviewer = user_model.NewGhostUser()
2323
}
2424

25+
apiTeam, err := ToTeam(r.ReviewerTeam)
26+
if err != nil {
27+
return nil, err
28+
}
29+
2530
result := &api.PullReview{
2631
ID: r.ID,
2732
Reviewer: ToUser(r.Reviewer, doer),
28-
ReviewerTeam: ToTeam(r.ReviewerTeam),
33+
ReviewerTeam: apiTeam,
2934
State: api.ReviewStateUnknown,
3035
Body: r.Content,
3136
CommitID: r.CommitID,

‎modules/convert/repository.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,7 @@ func innerToRepo(repo *repo_model.Repository, mode perm.AccessMode, isParent boo
186186

187187
// ToRepoTransfer convert a models.RepoTransfer to a structs.RepeTransfer
188188
func ToRepoTransfer(t *models.RepoTransfer) *api.RepoTransfer {
189-
var teams []*api.Team
190-
for _, v := range t.Teams {
191-
teams = append(teams, ToTeam(v))
192-
}
189+
teams, _ := ToTeams(t.Teams, false)
193190

194191
return &api.RepoTransfer{
195192
Doer: ToUser(t.Doer, nil),

‎routers/api/v1/org/team.go

Lines changed: 29 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,10 @@ func ListTeams(ctx *context.APIContext) {
5858
return
5959
}
6060

61-
apiTeams := make([]*api.Team, len(teams))
62-
for i := range teams {
63-
if err := teams[i].GetUnits(); err != nil {
64-
ctx.Error(http.StatusInternalServerError, "GetUnits", err)
65-
return
66-
}
67-
68-
apiTeams[i] = convert.ToTeam(teams[i])
61+
apiTeams, err := convert.ToTeams(teams, false)
62+
if err != nil {
63+
ctx.Error(http.StatusInternalServerError, "ConvertToTeams", err)
64+
return
6965
}
7066

7167
ctx.SetTotalCountHeader(count)
@@ -101,25 +97,10 @@ func ListUserTeams(ctx *context.APIContext) {
10197
return
10298
}
10399

104-
cache := make(map[int64]*api.Organization)
105-
apiTeams := make([]*api.Team, len(teams))
106-
for i := range teams {
107-
apiOrg, ok := cache[teams[i].OrgID]
108-
if !ok {
109-
org, err := organization.GetOrgByID(teams[i].OrgID)
110-
if err != nil {
111-
ctx.Error(http.StatusInternalServerError, "GetUserByID", err)
112-
return
113-
}
114-
apiOrg = convert.ToOrganization(org)
115-
cache[teams[i].OrgID] = apiOrg
116-
}
117-
if err := teams[i].GetUnits(); err != nil {
118-
ctx.Error(http.StatusInternalServerError, "teams[i].GetUnits()", err)
119-
return
120-
}
121-
apiTeams[i] = convert.ToTeam(teams[i])
122-
apiTeams[i].Organization = apiOrg
100+
apiTeams, err := convert.ToTeams(teams, true)
101+
if err != nil {
102+
ctx.Error(http.StatusInternalServerError, "ConvertToTeams", err)
103+
return
123104
}
124105

125106
ctx.SetTotalCountHeader(count)
@@ -144,12 +125,13 @@ func GetTeam(ctx *context.APIContext) {
144125
// "200":
145126
// "$ref": "#/responses/Team"
146127

147-
if err := ctx.Org.Team.GetUnits(); err != nil {
148-
ctx.Error(http.StatusInternalServerError, "team.GetUnits", err)
128+
apiTeam, err := convert.ToTeam(ctx.Org.Team)
129+
if err != nil {
130+
ctx.InternalServerError(err)
149131
return
150132
}
151133

152-
ctx.JSON(http.StatusOK, convert.ToTeam(ctx.Org.Team))
134+
ctx.JSON(http.StatusOK, apiTeam)
153135
}
154136

155137
func attachTeamUnits(team *organization.Team, units []string) {
@@ -241,7 +223,12 @@ func CreateTeam(ctx *context.APIContext) {
241223
return
242224
}
243225

244-
ctx.JSON(http.StatusCreated, convert.ToTeam(team))
226+
apiTeam, err := convert.ToTeam(team)
227+
if err != nil {
228+
ctx.InternalServerError(err)
229+
return
230+
}
231+
ctx.JSON(http.StatusCreated, apiTeam)
245232
}
246233

247234
// EditTeam api for edit a team
@@ -318,7 +305,13 @@ func EditTeam(ctx *context.APIContext) {
318305
ctx.Error(http.StatusInternalServerError, "EditTeam", err)
319306
return
320307
}
321-
ctx.JSON(http.StatusOK, convert.ToTeam(team))
308+
309+
apiTeam, err := convert.ToTeam(team)
310+
if err != nil {
311+
ctx.InternalServerError(err)
312+
return
313+
}
314+
ctx.JSON(http.StatusOK, apiTeam)
322315
}
323316

324317
// DeleteTeam api for delete a team
@@ -782,17 +775,10 @@ func SearchTeam(ctx *context.APIContext) {
782775
return
783776
}
784777

785-
apiTeams := make([]*api.Team, len(teams))
786-
for i := range teams {
787-
if err := teams[i].GetUnits(); err != nil {
788-
log.Error("Team GetUnits failed: %v", err)
789-
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
790-
"ok": false,
791-
"error": "SearchTeam failed to get units",
792-
})
793-
return
794-
}
795-
apiTeams[i] = convert.ToTeam(teams[i])
778+
apiTeams, err := convert.ToTeams(teams, false)
779+
if err != nil {
780+
ctx.InternalServerError(err)
781+
return
796782
}
797783

798784
ctx.SetLinkHeader(int(maxResults), listOptions.PageSize)

‎routers/api/v1/repo/teams.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"code.gitea.io/gitea/models/organization"
1313
"code.gitea.io/gitea/modules/context"
1414
"code.gitea.io/gitea/modules/convert"
15-
api "code.gitea.io/gitea/modules/structs"
1615
)
1716

1817
// ListTeams list a repository's teams
@@ -48,14 +47,10 @@ func ListTeams(ctx *context.APIContext) {
4847
return
4948
}
5049

51-
apiTeams := make([]*api.Team, len(teams))
52-
for i := range teams {
53-
if err := teams[i].GetUnits(); err != nil {
54-
ctx.Error(http.StatusInternalServerError, "GetUnits", err)
55-
return
56-
}
57-
58-
apiTeams[i] = convert.ToTeam(teams[i])
50+
apiTeams, err := convert.ToTeams(teams, false)
51+
if err != nil {
52+
ctx.InternalServerError(err)
53+
return
5954
}
6055

6156
ctx.JSON(http.StatusOK, apiTeams)
@@ -103,11 +98,11 @@ func IsTeam(ctx *context.APIContext) {
10398
}
10499

105100
if models.HasRepository(team, ctx.Repo.Repository.ID) {
106-
if err := team.GetUnits(); err != nil {
107-
ctx.Error(http.StatusInternalServerError, "GetUnits", err)
101+
apiTeam, err := convert.ToTeam(team)
102+
if err != nil {
103+
ctx.InternalServerError(err)
108104
return
109105
}
110-
apiTeam := convert.ToTeam(team)
111106
ctx.JSON(http.StatusOK, apiTeam)
112107
return
113108
}

0 commit comments

Comments
 (0)