Adds checking of reserved keywords against team names

[ghost]

This fixes and solves gogit/gogs#3789

[tboerger]

LGTM but needs to be rebased

[tboerger]

Please reopen against master.

[strk]

Would it make sense for the empty name case to be handled by the IsUsableTeamName directly ?
Also, it looks like the IsUsableTeamName function is a perfect candidate for an automated test, how about adding an models/org_team_test.go ?

[strk]

Full link to original issue: gogs/gogs#3789

[thibaultmeyer]

I agree with @strk , if the team name is empty, don't enter on IsUsableTeamName function. If empty, you are sure at 100% it was an invalid team name, no need to check if is forbidden or not.

Maybe forbidden team names could be move to app.ini file (or a special file just for this purpose) or into database to allow users to edit list without recompile project.

[lunny]

case sensitive?

[ghost]

I did it in the same manner as models/user.go#L504:L508.

[bkcsoft]

Macaron isn't (apparently) case insensitive ( test by going to /org/CREATE instead of /org/create )

[tboerger]

@lunny is right, you should always downcase the name.

[tboerger]

Maybe forbidden team names could be move to app.ini file (or a special file just for this purpose) or into database to allow users to edit list without recompile project.

This list contains only names that are forbidden to prevent issues with the routes.

[tboerger]

Yes, the others are right, move that below the if len(t.Name) == 0 { check.

[strk]

My suggestion was actually to move the len(t.Name) == 0 check inside IsUsableTeamName (as empty is clearly not a usable team name)

[ghost]

@tboerger Fixed.

[codecov-io]

Current coverage is 2.18% (diff: 0.00%)

Merging #22 into master will decrease coverage by <.01%

@@            master       #22   diff @@
========================================
  Files           31        31          
  Lines         7508      7516     +8   
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits           164       164          
- Misses        7327      7335     +8   
  Partials        17        17          

Powered by Codecov. Last update 03902bb...6662e9a

[strk]

LGTM

[tboerger]

@lunny is right, you should always downcase the name.

[tboerger]

for _, reservedName := range reservedTeamNames {
  if reservedName == strings.ToLower(name) {

  }
}

[tboerger]

Sorry, but after seeing the comment of @lunny I changed my opinion :D

[strk]

@tboerger changed your mind so you want to Dismiss review ?
@LefsFlarey we still need tests, according to codecov (and it'd be worth a rebase)

[strk]

Assuming everyone is happy with this, merging (since github lets me)

[tboerger]

I didn't realized that before, but why have that been merged when there are still requests for changes left? I requested to do always a downcase check...