#2497 incorrect error handle for team name

README.md

@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
 
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
 
-##### Current version: 0.8.24
+##### Current version: 0.8.25
 
 | Web | UI  | Preview  |
 |:-------------:|:-------:|:-------:|

conf/locale/locale_en-US.ini

@@ -203,7 +203,6 @@ repo_name_been_taken = Repository name has already been taken.
 org_name_been_taken = Organization name has already been taken.
 team_name_been_taken = Team name has already been taken.
 email_been_used = Email address has already been used.
-illegal_team_name = Team name contains illegal characters.
 username_password_incorrect = Username or password is not correct.
 enterred_invalid_repo_name = Please make sure that the repository name you entered is correct.
 enterred_invalid_owner_name = Please make sure that the owner name you entered is correct.

gogs.go

@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.8.24.0129"
+const APP_VER = "0.8.25.0129"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())

models/error.go

@@ -559,5 +559,26 @@ func IsErrAuthenticationNotExist(err error) bool {
 }
 
 func (err ErrAuthenticationNotExist) Error() string {
-	return fmt.Sprintf("Authentication does not exist [id: %d]", err.ID)
+	return fmt.Sprintf("authentication does not exist [id: %d]", err.ID)
+}
+
+// ___________
+// \__    ___/___ _____    _____
+//   |    |_/ __ \\__  \  /     \
+//   |    |\  ___/ / __ \|  Y Y  \
+//   |____| \___  >____  /__|_|  /
+//              \/     \/      \/
+
+type ErrTeamAlreadyExist struct {
+	OrgID int64
+	Name  string
+}
+
+func IsErrTeamAlreadyExist(err error) bool {
+	_, ok := err.(ErrTeamAlreadyExist)
+	return ok
+}
+
+func (err ErrTeamAlreadyExist) Error() string {
+	return fmt.Sprintf("team already exists [org_id: %d, name: %s]", err.OrgID, err.Name)
 }

models/org.go

@@ -14,10 +14,8 @@ import (
 )
 
 var (
-	ErrOrgNotExist      = errors.New("Organization does not exist")
+	ErrOrgNotExist  = errors.New("Organization does not exist")
-	ErrTeamAlreadyExist = errors.New("Team already exist")
+	ErrTeamNotExist = errors.New("Team does not exist")
-	ErrTeamNotExist     = errors.New("Team does not exist")
-	ErrTeamNameIllegal  = errors.New("Team name contains illegal characters")
 )
 
 // IsOwnedBy returns true if given user is in the owner team.
@@ -598,9 +596,9 @@ func (t *Team) RemoveRepository(repoID int64) error {
 
 // NewTeam creates a record of new team.
 // It's caller's responsibility to assign organization ID.
-func NewTeam(t *Team) (err error) {
+func NewTeam(t *Team) error {
-	if err = IsUsableName(t.Name); err != nil {
+	if len(t.Name) == 0 {
-		return err
+		return errors.New("empty team name")
 	}
 
 	has, err := x.Id(t.OrgID).Get(new(User))
@@ -615,7 +613,7 @@ func NewTeam(t *Team) (err error) {
 	if err != nil {
 		return err
 	} else if has {
-		return ErrTeamAlreadyExist
+		return ErrTeamAlreadyExist{t.OrgID, t.LowerName}
 	}
 
 	sess := x.NewSession()
@@ -674,8 +672,8 @@ func GetTeamById(teamId int64) (*Team, error) {
 
 // UpdateTeam updates information of team.
 func UpdateTeam(t *Team, authChanged bool) (err error) {
-	if err = IsUsableName(t.Name); err != nil {
+	if len(t.Name) == 0 {
-		return err
+		return errors.New("empty team name")
 	}
 
 	if len(t.Description) > 255 {
@@ -689,6 +687,13 @@ func UpdateTeam(t *Team, authChanged bool) (err error) {
 	}
 
 	t.LowerName = strings.ToLower(t.Name)
+	has, err := x.Where("org_id=?", t.OrgID).And("lower_name=?", t.LowerName).And("id!=?", t.ID).Get(new(Team))
+	if err != nil {
+		return err
+	} else if has {
+		return ErrTeamAlreadyExist{t.OrgID, t.LowerName}
+	}
+
 	if _, err = sess.Id(t.ID).AllCols().Update(t); err != nil {
 		return fmt.Errorf("update: %v", err)
 	}

modules/auth/org.go

@@ -45,9 +45,9 @@ func (f *UpdateOrgSettingForm) Validate(ctx *macaron.Context, errs binding.Error
 //              \/     \/      \/
 
 type CreateTeamForm struct {
-	TeamName    string `form:"team_name" binding:"Required;AlphaDashDot;MaxSize(30)"`
+	TeamName    string `binding:"Required;AlphaDashDot;MaxSize(30)"`
-	Description string `form:"desc" binding:"MaxSize(255)"`
+	Description string `binding:"MaxSize(255)"`
-	Permission  string `form:"permission"`
+	Permission  string
 }
 
 func (f *CreateTeamForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors {

routers/org/teams.go

@@ -157,12 +157,6 @@ func NewTeamPost(ctx *middleware.Context, form auth.CreateTeamForm) {
 	ctx.Data["Title"] = ctx.Org.Organization.FullName
 	ctx.Data["PageIsOrgTeams"] = true
 	ctx.Data["PageIsOrgTeamsNew"] = true
-	ctx.Data["Team"] = &models.Team{}
-
-	if ctx.HasError() {
-		ctx.HTML(200, TEAM_NEW)
-		return
-	}
 
 	// Validate permission level.
 	var auth models.AccessMode
@@ -178,28 +172,30 @@ func NewTeamPost(ctx *middleware.Context, form auth.CreateTeamForm) {
 		return
 	}
 
-	org := ctx.Org.Organization
-
 	t := &models.Team{
-		OrgID:       org.Id,
+		OrgID:       ctx.Org.Organization.Id,
 		Name:        form.TeamName,
 		Description: form.Description,
 		Authorize:   auth,
 	}
+	ctx.Data["Team"] = t
+
+	if ctx.HasError() {
+		ctx.HTML(200, TEAM_NEW)
+		return
+	}
+
 	if err := models.NewTeam(t); err != nil {
-		switch err {
+		ctx.Data["Err_TeamName"] = true
-		case models.ErrTeamNameIllegal:
+		switch {
-			ctx.Data["Err_TeamName"] = true
+		case models.IsErrTeamAlreadyExist(err):
-			ctx.RenderWithErr(ctx.Tr("form.illegal_team_name"), TEAM_NEW, &form)
-		case models.ErrTeamAlreadyExist:
-			ctx.Data["Err_TeamName"] = true
 			ctx.RenderWithErr(ctx.Tr("form.team_name_been_taken"), TEAM_NEW, &form)
 		default:
 			ctx.Handle(500, "NewTeam", err)
 		}
 		return
 	}
-	log.Trace("Team created: %s/%s", org.Name, t.Name)
+	log.Trace("Team created: %s/%s", ctx.Org.Organization.Name, t.Name)
 	ctx.Redirect(ctx.Org.OrgLink + "/teams/" + t.LowerName)
 }
 
@@ -235,8 +231,7 @@ func EditTeamPost(ctx *middleware.Context, form auth.CreateTeamForm) {
 	t := ctx.Org.Team
 	ctx.Data["Title"] = ctx.Org.Organization.FullName
 	ctx.Data["PageIsOrgTeams"] = true
-	ctx.Data["team_name"] = t.Name
+	ctx.Data["Team"] = t
-	ctx.Data["desc"] = t.Description
 
 	if ctx.HasError() {
 		ctx.HTML(200, TEAM_NEW)
@@ -267,10 +262,11 @@ func EditTeamPost(ctx *middleware.Context, form auth.CreateTeamForm) {
 	}
 	t.Description = form.Description
 	if err := models.UpdateTeam(t, isAuthChanged); err != nil {
-		if err == models.ErrTeamNameIllegal {
+		ctx.Data["Err_TeamName"] = true
-			ctx.Data["Err_TeamName"] = true
+		switch {
-			ctx.RenderWithErr(ctx.Tr("form.illegal_team_name"), TEAM_NEW, &form)
+		case models.IsErrTeamAlreadyExist(err):
-		} else {
+			ctx.RenderWithErr(ctx.Tr("form.team_name_been_taken"), TEAM_NEW, &form)
+		default:
 			ctx.Handle(500, "UpdateTeam", err)
 		}
 		return

templates/.VERSION

@@ -1 +1 @@
-0.8.24.0129
+0.8.25.0129

templates/org/team/new.tmpl

@@ -13,14 +13,14 @@
 					<div class="required field {{if .Err_TeamName}}error{{end}}">
 						<label for="team_name">{{.i18n.Tr "org.team_name"}}</label>
 						{{if eq .Team.LowerName "owners"}}
-							<input type="hidden" name="team_name" value="{{.team_name}}">
+							<input type="hidden" name="team_name" value="{{.Team.Name}}">
 						{{end}}
-						<input id="team_name" name="team_name" value="{{.team_name}}" required {{if eq .Team.LowerName "owners"}}disabled{{end}} autofocus>
+						<input id="team_name" name="team_name" value="{{.Team.Name}}" required {{if eq .Team.LowerName "owners"}}disabled{{end}} autofocus>
 						<span class="help">{{.i18n.Tr "org.team_name_helper"}}</span>
 					</div>
 					<div class="field {{if .Err_Description}}error{{end}}">
-						<label for="desc">{{.i18n.Tr "org.team_desc"}}</label>
+						<label for="description">{{.i18n.Tr "org.team_desc"}}</label>
-						<input id="desc" name="desc" value="{{.desc}}">
+						<input id="description" name="description" value="{{.Team.Description}}">
 						<span class="help">{{.i18n.Tr "org.team_desc_helper"}}</span>
 					</div>
 					{{if not (eq .Team.LowerName "owners")}}