From 112a7cab31df083983a664e252efcfca4ecb0d7a Mon Sep 17 00:00:00 2001 From: Unknwon Date: Fri, 29 Jan 2016 17:06:14 -0500 Subject: [PATCH] #2497 incorrect error handle for team name --- README.md | 2 +- conf/locale/locale_en-US.ini | 1 - gogs.go | 2 +- models/error.go | 23 ++++++++++++++++++++- models/org.go | 25 +++++++++++++--------- modules/auth/org.go | 6 +++--- routers/org/teams.go | 40 ++++++++++++++++-------------------- templates/.VERSION | 2 +- templates/org/team/new.tmpl | 8 ++++---- 9 files changed, 65 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 275c2b27753..41fe2ebee79 100644 --- a/README.md +++ b/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 | |:-------------:|:-------:|:-------:| diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index 9cdaddf8483..b9d79716e27 100644 --- a/conf/locale/locale_en-US.ini +++ b/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. diff --git a/gogs.go b/gogs.go index d1091b2c79f..01bf1582907 100644 --- a/gogs.go +++ b/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()) diff --git a/models/error.go b/models/error.go index 83a24e7f4e1..8e2048ded9e 100644 --- a/models/error.go +++ b/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) } diff --git a/models/org.go b/models/org.go index fa26b59e660..b8836c349f3 100644 --- a/models/org.go +++ b/models/org.go @@ -14,10 +14,8 @@ import ( ) var ( - ErrOrgNotExist = errors.New("Organization does not exist") - ErrTeamAlreadyExist = errors.New("Team already exist") - ErrTeamNotExist = errors.New("Team does not exist") - ErrTeamNameIllegal = errors.New("Team name contains illegal characters") + ErrOrgNotExist = errors.New("Organization does not exist") + ErrTeamNotExist = errors.New("Team does not exist") ) // 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) { - if err = IsUsableName(t.Name); err != nil { - return err +func NewTeam(t *Team) error { + if len(t.Name) == 0 { + 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 { - return err + if len(t.Name) == 0 { + 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) } diff --git a/modules/auth/org.go b/modules/auth/org.go index 8af4ad55d81..53ef6245d9e 100644 --- a/modules/auth/org.go +++ b/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)"` - Description string `form:"desc" binding:"MaxSize(255)"` - Permission string `form:"permission"` + TeamName string `binding:"Required;AlphaDashDot;MaxSize(30)"` + Description string `binding:"MaxSize(255)"` + Permission string } func (f *CreateTeamForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { diff --git a/routers/org/teams.go b/routers/org/teams.go index 2dd3c1981c2..b2128baab6c 100644 --- a/routers/org/teams.go +++ b/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 { - case models.ErrTeamNameIllegal: - ctx.Data["Err_TeamName"] = true - ctx.RenderWithErr(ctx.Tr("form.illegal_team_name"), TEAM_NEW, &form) - case models.ErrTeamAlreadyExist: - ctx.Data["Err_TeamName"] = true + ctx.Data["Err_TeamName"] = true + switch { + case models.IsErrTeamAlreadyExist(err): 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["desc"] = t.Description + ctx.Data["Team"] = t 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.RenderWithErr(ctx.Tr("form.illegal_team_name"), TEAM_NEW, &form) - } else { + ctx.Data["Err_TeamName"] = true + switch { + case models.IsErrTeamAlreadyExist(err): + ctx.RenderWithErr(ctx.Tr("form.team_name_been_taken"), TEAM_NEW, &form) + default: ctx.Handle(500, "UpdateTeam", err) } return diff --git a/templates/.VERSION b/templates/.VERSION index c9116fb65b9..43ad4d3d125 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.8.24.0129 \ No newline at end of file +0.8.25.0129 \ No newline at end of file diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 6293efb62ef..020eed384dc 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -13,14 +13,14 @@
{{if eq .Team.LowerName "owners"}} - + {{end}} - + {{.i18n.Tr "org.team_name_helper"}}
- - + + {{.i18n.Tr "org.team_desc_helper"}}
{{if not (eq .Team.LowerName "owners")}}