From b49e4a848b36480106a13ec7da397c641b67cb3f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 31 Dec 2019 01:32:07 +0100 Subject: [PATCH 01/11] API: orgEditTeam make Fields optional --- modules/structs/org_team.go | 4 ++-- routers/api/v1/org/team.go | 38 +++++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/modules/structs/org_team.go b/modules/structs/org_team.go index 16f83823d66b5..3c4f26c0d8f38 100644 --- a/modules/structs/org_team.go +++ b/modules/structs/org_team.go @@ -35,12 +35,12 @@ type CreateTeamOption struct { // EditTeamOption options for editing a team type EditTeamOption struct { // required: true - Name string `json:"name" binding:"Required;AlphaDashDot;MaxSize(30)"` + Name string `json:"name" binding:"AlphaDashDot;MaxSize(30)"` Description string `json:"description" binding:"MaxSize(255)"` IncludesAllRepositories bool `json:"includes_all_repositories"` // enum: read,write,admin Permission string `json:"permission"` // example: ["repo.code","repo.issues","repo.ext_issues","repo.wiki","repo.pulls","repo.releases","repo.ext_wiki"] Units []string `json:"units"` - CanCreateOrgRepo bool `json:"can_create_org_repo"` + CanCreateOrgRepo *bool `json:"can_create_org_repo"` } diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 73714e6a66356..395664eb2a868 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -192,17 +192,28 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { // "$ref": "#/responses/Team" team := ctx.Org.Team - team.Description = form.Description - unitTypes := models.FindUnitTypes(form.Units...) - team.CanCreateOrgRepo = form.CanCreateOrgRepo + if err := team.GetUnits(); err != nil { + ctx.InternalServerError(err) + } + + if form.CanCreateOrgRepo != nil { + team.CanCreateOrgRepo = *form.CanCreateOrgRepo + } + + if form.Name != "" { + team.Name = form.Name + } + + if form.Description != "" { + team.Description = form.Description + } isAuthChanged := false isIncludeAllChanged := false - if !team.IsOwnerTeam() { + if !team.IsOwnerTeam() && form.Permission != "" { // Validate permission level. auth := models.ParseAccessMode(form.Permission) - team.Name = form.Name if team.Authorize != auth { isAuthChanged = true team.Authorize = auth @@ -215,14 +226,17 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { } if team.Authorize < models.AccessModeOwner { - var units = make([]*models.TeamUnit, 0, len(form.Units)) - for _, tp := range unitTypes { - units = append(units, &models.TeamUnit{ - OrgID: ctx.Org.Team.OrgID, - Type: tp, - }) + if len(form.Units) > 0 { + var units = make([]*models.TeamUnit, 0, len(form.Units)) + unitTypes := models.FindUnitTypes(form.Units...) + for _, tp := range unitTypes { + units = append(units, &models.TeamUnit{ + OrgID: ctx.Org.Team.OrgID, + Type: tp, + }) + } + team.Units = units } - team.Units = units } if err := models.UpdateTeam(team, isAuthChanged, isIncludeAllChanged); err != nil { From ecc15a50260e22f059b4ffcaa48bbabdd8f09d87 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 2 Jan 2020 06:36:01 +0100 Subject: [PATCH 02/11] add TestCase --- integrations/api_team_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/integrations/api_team_test.go b/integrations/api_team_test.go index be56e37edfebb..8d073e4ce38f7 100644 --- a/integrations/api_team_test.go +++ b/integrations/api_team_test.go @@ -86,12 +86,22 @@ func TestAPITeam(t *testing.T) { checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) + // Edit team without Name + teamToEditDesc := &api.EditTeamOption{Description: "first team"} + req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEditDesc) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &apiTeam) + checkTeamResponse(t, &apiTeam, teamToEdit.Name, teamToEditDesc.Description, teamToEdit.IncludesAllRepositories, + teamToEdit.Permission, teamToEdit.Units) + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEditDesc.Description, teamToEdit.IncludesAllRepositories, + teamToEdit.Permission, teamToEdit.Units) + // Read team. teamRead := models.AssertExistsAndLoadBean(t, &models.Team{ID: teamID}).(*models.Team) req = NewRequestf(t, "GET", "/api/v1/teams/%d?token="+token, teamID) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiTeam) - checkTeamResponse(t, &apiTeam, teamRead.Name, teamRead.Description, teamRead.IncludesAllRepositories, + checkTeamResponse(t, &apiTeam, teamRead.Name, teamToEditDesc.Description, teamRead.IncludesAllRepositories, teamRead.Authorize.String(), teamRead.GetUnitNames()) // Delete team. From 0c76d5ca86f6c0dd4f72628a634e4b77ef1badd1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 2 Jan 2020 08:11:24 +0100 Subject: [PATCH 03/11] Update integrations/api_team_test.go --- integrations/api_team_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_team_test.go b/integrations/api_team_test.go index 8d073e4ce38f7..1fc1860b7e667 100644 --- a/integrations/api_team_test.go +++ b/integrations/api_team_test.go @@ -86,7 +86,7 @@ func TestAPITeam(t *testing.T) { checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) - // Edit team without Name + // Edit team Description only teamToEditDesc := &api.EditTeamOption{Description: "first team"} req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEditDesc) resp = session.MakeRequest(t, req, http.StatusOK) From a9a0f9b53fa91f6e6550d66d2b8c79a6eeeb4a46 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 2 Jan 2020 14:58:57 +0100 Subject: [PATCH 04/11] suggestions from lafriks use len() to check if string is empty Co-Authored-By: Lauris BH --- routers/api/v1/org/team.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 395664eb2a868..85e2f1186eda1 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -200,17 +200,17 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { team.CanCreateOrgRepo = *form.CanCreateOrgRepo } - if form.Name != "" { + if len(form.Name) != 0 { team.Name = form.Name } - if form.Description != "" { + if len(form.Description) != 0 { team.Description = form.Description } isAuthChanged := false isIncludeAllChanged := false - if !team.IsOwnerTeam() && form.Permission != "" { + if !team.IsOwnerTeam() && len(form.Permission) != 0 { // Validate permission level. auth := models.ParseAccessMode(form.Permission) From 322d657f449e0258666b369ea29f0f51c2bf9336 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 03:43:32 +0100 Subject: [PATCH 05/11] change ... --- modules/structs/org_team.go | 6 +++--- routers/api/v1/org/team.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/structs/org_team.go b/modules/structs/org_team.go index 3c4f26c0d8f38..a742b7b224f91 100644 --- a/modules/structs/org_team.go +++ b/modules/structs/org_team.go @@ -35,9 +35,9 @@ type CreateTeamOption struct { // EditTeamOption options for editing a team type EditTeamOption struct { // required: true - Name string `json:"name" binding:"AlphaDashDot;MaxSize(30)"` - Description string `json:"description" binding:"MaxSize(255)"` - IncludesAllRepositories bool `json:"includes_all_repositories"` + Name string `json:"name" binding:"AlphaDashDot;MaxSize(30)"` + Description *string `json:"description" binding:"MaxSize(255)"` + IncludesAllRepositories *bool `json:"includes_all_repositories"` // enum: read,write,admin Permission string `json:"permission"` // example: ["repo.code","repo.issues","repo.ext_issues","repo.wiki","repo.pulls","repo.releases","repo.ext_wiki"] diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 85e2f1186eda1..7192d4f0f24bb 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -204,8 +204,8 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { team.Name = form.Name } - if len(form.Description) != 0 { - team.Description = form.Description + if form.Description != nil { + team.Description = *form.Description } isAuthChanged := false @@ -219,9 +219,9 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { team.Authorize = auth } - if team.IncludesAllRepositories != form.IncludesAllRepositories { + if form.IncludesAllRepositories != nil { isIncludeAllChanged = true - team.IncludesAllRepositories = form.IncludesAllRepositories + team.IncludesAllRepositories = *form.IncludesAllRepositories } } From bae6724216f5b3bf367910e15a9b3d0b4197d495 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 03:45:41 +0100 Subject: [PATCH 06/11] use Where not ID to get mssql --- models/org_team.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/org_team.go b/models/org_team.go index 63c6e1163685e..95e383699c102 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -590,7 +590,7 @@ func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) { return ErrTeamAlreadyExist{t.OrgID, t.LowerName} } - if _, err = sess.ID(t.ID).AllCols().Update(t); err != nil { + if _, err = sess.Where("team.id = ?", t.ID).AllCols().Update(t); err != nil { return fmt.Errorf("update: %v", err) } From ad12e31a3629ea5d2eba11bcb365626df5e68d17 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 04:03:19 +0100 Subject: [PATCH 07/11] add return and code format --- routers/api/v1/org/team.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 7192d4f0f24bb..446287a343773 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -194,13 +194,14 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { team := ctx.Org.Team if err := team.GetUnits(); err != nil { ctx.InternalServerError(err) + return } if form.CanCreateOrgRepo != nil { team.CanCreateOrgRepo = *form.CanCreateOrgRepo } - if len(form.Name) != 0 { + if len(form.Name) > 0 { team.Name = form.Name } From f52f73661871fa91e3d09c5b4d328d0ad4034fb6 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 04:39:36 +0100 Subject: [PATCH 08/11] fix test --- integrations/api_team_test.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/integrations/api_team_test.go b/integrations/api_team_test.go index 1fc1860b7e667..7972504052363 100644 --- a/integrations/api_team_test.go +++ b/integrations/api_team_test.go @@ -72,28 +72,30 @@ func TestAPITeam(t *testing.T) { // Edit team. teamToEdit := &api.EditTeamOption{ - Name: "teamone", - Description: "team 1", - IncludesAllRepositories: false, - Permission: "admin", - Units: []string{"repo.code", "repo.pulls", "repo.releases"}, + Name: "teamone", + Permission: "admin", + Units: []string{"repo.code", "repo.pulls", "repo.releases"}, } + *teamToEdit.Description = "team 1" + *teamToEdit.IncludesAllRepositories = false + req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEdit) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiTeam) - checkTeamResponse(t, &apiTeam, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories, + checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories, + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) // Edit team Description only - teamToEditDesc := &api.EditTeamOption{Description: "first team"} + var teamToEditDesc api.EditTeamOption + *teamToEditDesc.Description = "first team" req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEditDesc) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiTeam) - checkTeamResponse(t, &apiTeam, teamToEdit.Name, teamToEditDesc.Description, teamToEdit.IncludesAllRepositories, + checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEditDesc.Description, teamToEdit.IncludesAllRepositories, + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, teamToEdit.Permission, teamToEdit.Units) // Read team. @@ -101,7 +103,7 @@ func TestAPITeam(t *testing.T) { req = NewRequestf(t, "GET", "/api/v1/teams/%d?token="+token, teamID) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiTeam) - checkTeamResponse(t, &apiTeam, teamRead.Name, teamToEditDesc.Description, teamRead.IncludesAllRepositories, + checkTeamResponse(t, &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, teamRead.Authorize.String(), teamRead.GetUnitNames()) // Delete team. From 02a87c91977e5cd4f0887d681146ef450e0f7b60 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 04:48:08 +0100 Subject: [PATCH 09/11] fix test ... null pointer exept --- integrations/api_team_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/integrations/api_team_test.go b/integrations/api_team_test.go index 7972504052363..d89385447092f 100644 --- a/integrations/api_team_test.go +++ b/integrations/api_team_test.go @@ -71,13 +71,15 @@ func TestAPITeam(t *testing.T) { teamID := apiTeam.ID // Edit team. + editDescription := "team 1" + editFalse := false teamToEdit := &api.EditTeamOption{ - Name: "teamone", - Permission: "admin", - Units: []string{"repo.code", "repo.pulls", "repo.releases"}, + Name: "teamone", + Description: &editDescription, + Permission: "admin", + IncludesAllRepositories: &editFalse, + Units: []string{"repo.code", "repo.pulls", "repo.releases"}, } - *teamToEdit.Description = "team 1" - *teamToEdit.IncludesAllRepositories = false req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEdit) resp = session.MakeRequest(t, req, http.StatusOK) @@ -88,8 +90,8 @@ func TestAPITeam(t *testing.T) { teamToEdit.Permission, teamToEdit.Units) // Edit team Description only - var teamToEditDesc api.EditTeamOption - *teamToEditDesc.Description = "first team" + editDescription = "first team" + teamToEditDesc := api.EditTeamOption{Description: &editDescription} req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEditDesc) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiTeam) From c2631197bd40c1f3f6e3d138e3f20f084d10982a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 05:55:17 +0100 Subject: [PATCH 10/11] update specific colums --- models/org_team.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/org_team.go b/models/org_team.go index 95e383699c102..e4a146837fe9d 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -590,7 +590,8 @@ func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) { return ErrTeamAlreadyExist{t.OrgID, t.LowerName} } - if _, err = sess.Where("team.id = ?", t.ID).AllCols().Update(t); err != nil { + if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", + "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { return fmt.Errorf("update: %v", err) } From 6d368619623c559b39f89c2bf81c7987c6cb440e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 4 Jan 2020 06:17:26 +0100 Subject: [PATCH 11/11] only specific colums too --- models/org_team.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index e4a146837fe9d..0c0a1e7b79734 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -606,8 +606,7 @@ func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) { Delete(new(TeamUnit)); err != nil { return err } - - if _, err = sess.Insert(&t.Units); err != nil { + if _, err = sess.Cols("org_id", "team_id", "type").Insert(&t.Units); err != nil { errRollback := sess.Rollback() if errRollback != nil { log.Error("UpdateTeam sess.Rollback: %v", errRollback)