[API] orgEditTeam make Fields optional (#9556)

* API: orgEditTeam make Fields optional

* add TestCase

* Update integrations/api_team_test.go

* suggestions from lafriks

use len() to check if string is empty

Co-Authored-By: Lauris BH <[email protected]>

* change ...

* use Where not ID to get mssql

* add return and code format

* fix test

* fix test ... null pointer exept

* update specific colums

* only specific colums too

Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>

Author: 3 people

integrations/api_team_test.go

@@ -71,27 +71,41 @@ func TestAPITeam(t *testing.T) {
 	teamID := apiTeam.ID
 
 	// Edit team.
+	editDescription := "team 1"
+	editFalse := false
 	teamToEdit := &api.EditTeamOption{
 		Name:                    "teamone",
-		Description:             "team 1",
-		IncludesAllRepositories: false,
+		Description:             &editDescription,
 		Permission:              "admin",
+		IncludesAllRepositories: &editFalse,
 		Units:                   []string{"repo.code", "repo.pulls", "repo.releases"},
 	}
+
 	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,
+		teamToEdit.Permission, teamToEdit.Units)
+
+	// Edit team Description only
+	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)
+	checkTeamResponse(t, &apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories,
 		teamToEdit.Permission, teamToEdit.Units)
-	checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories,
+	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.

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.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)
 	}
 
@@ -605,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)

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)"`
-	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"]
 	Units            []string `json:"units"`
-	CanCreateOrgRepo bool     `json:"can_create_org_repo"`
+	CanCreateOrgRepo *bool    `json:"can_create_org_repo"`
 }

routers/api/v1/org/team.go

@@ -192,37 +192,52 @@ 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)
+		return
+	}
+
+	if form.CanCreateOrgRepo != nil {
+		team.CanCreateOrgRepo = *form.CanCreateOrgRepo
+	}
+
+	if len(form.Name) > 0 {
+		team.Name = form.Name
+	}
+
+	if form.Description != nil {
+		team.Description = *form.Description
+	}
 
 	isAuthChanged := false
 	isIncludeAllChanged := false
-	if !team.IsOwnerTeam() {
+	if !team.IsOwnerTeam() && len(form.Permission) != 0 {
 		// Validate permission level.
 		auth := models.ParseAccessMode(form.Permission)
 
-		team.Name = form.Name
 		if team.Authorize != auth {
 			isAuthChanged = true
 			team.Authorize = auth
 		}
 
-		if team.IncludesAllRepositories != form.IncludesAllRepositories {
+		if form.IncludesAllRepositories != nil {
 			isIncludeAllChanged = true
-			team.IncludesAllRepositories = form.IncludesAllRepositories
+			team.IncludesAllRepositories = *form.IncludesAllRepositories
 		}
 	}
 
 	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 {