Remove unit types commits and settings (#2161)

* Remove unit types commits and settings

* Can not limit units in administrator teams

* Limit changing units only to teams with read and write access mode

* Small code optimization

10 files changed, 114 insertions, 77 deletions

diff --git a/models/access_test.go b/models/access_test.go
index 29d40d9586..59575acb7d 100644
--- a/models/access_test.go
+++ b/models/access_test.go
@@ -21,7 +21,7 @@ func TestAccessLevel(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	user2 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
+	user2 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
 	repo1 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 2, IsPrivate: false}).(*Repository)
 	repo2 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 3, IsPrivate: true}).(*Repository)
 
@@ -46,7 +46,7 @@ func TestHasAccess(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	user2 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
+	user2 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
 	repo1 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 2, IsPrivate: false}).(*Repository)
 	repo2 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 3, IsPrivate: true}).(*Repository)
 
diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml
index 5ea4076107..8722b248db 100644
--- a/models/fixtures/access.yml
+++ b/models/fixtures/access.yml
@@ -9,3 +9,9 @@
   user_id: 4
   repo_id: 4
   mode: 2 # write
+
+-
+  id: 3
+  user_id: 4
+  repo_id: 3
+  mode: 2 # write
diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml
index daa3480189..02daa48277 100644
--- a/models/fixtures/repo_unit.yml
+++ b/models/fixtures/repo_unit.yml
@@ -10,7 +10,7 @@
   id: 2
   repo_id: 1
   type: 2
-  index: 0
+  index: 1
   config: "{}"
   created_unix: 946684810
 
@@ -18,23 +18,23 @@
   id: 3
   repo_id: 1
   type: 3
-  index: 0
+  index: 2
   config: "{}"
   created_unix: 946684810
 
 -
   id: 4
   repo_id: 1
-  type: 5
-  index: 0
+  type: 4
+  index: 3
   config: "{}"
   created_unix: 946684810
 
 -
   id: 5
   repo_id: 1
-  type: 7
-  index: 0
+  type: 5
+  index: 4
   config: "{}"
   created_unix: 946684810
 
@@ -50,7 +50,7 @@
   id: 7
   repo_id: 3
   type: 2
-  index: 0
+  index: 1
   config: "{}"
   created_unix: 946684810
 
@@ -58,22 +58,22 @@
   id: 8
   repo_id: 3
   type: 3
-  index: 0
+  index: 2
   config: "{}"
   created_unix: 946684810
 
 -
   id: 9
   repo_id: 3
-  type: 5
-  index: 0
+  type: 4
+  index: 3
   config: "{}"
   created_unix: 946684810
 
 -
   id: 10
   repo_id: 3
-  type: 7
-  index: 0
+  type: 5
+  index: 4
   config: "{}"
   created_unix: 946684810
diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml
index 1b2a0d6811..795d5cda6c 100644
--- a/models/fixtures/team.yml
+++ b/models/fixtures/team.yml
@@ -6,7 +6,7 @@
   authorize: 4 # owner
   num_repos: 2
   num_members: 1
-  unit_types: '[1,2,3,4,5,6,7,8,9]'
+  unit_types: '[1,2,3,4,5,6,7]'
 
 -
   id: 2
@@ -16,7 +16,7 @@
   authorize: 2 # write
   num_repos: 1
   num_members: 2
-  unit_types: '[1,2,3,4,5,6,7,8,9]'
+  unit_types: '[1,2,3,4,5,6,7]'
 
 -
   id: 3
@@ -26,7 +26,7 @@
   authorize: 4 # owner
   num_repos: 0
   num_members: 1
-  unit_types: '[1,2,3,4,5,6,7,8,9]'
+  unit_types: '[1,2,3,4,5,6,7]'
 
 -
   id: 4
@@ -36,4 +36,4 @@
   authorize: 4 # owner
   num_repos: 0
   num_members: 1
-  unit_types: '[1,2,3,4,5,6,7,8,9]'
\ No newline at end of file
+  unit_types: '[1,2,3,4,5,6,7]'
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index e5b6473687..45c410c4d4 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -124,6 +124,8 @@ var migrations = []Migration{
 	NewMigration("regenerate git hooks", regenerateGitHooks36),
 	// v37 -> v38
 	NewMigration("unescape user full names", unescapeUserFullNames),
+	// v38 -> v39
+	NewMigration("remove commits and settings unit types", removeCommitsUnitType),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v16.go b/models/migrations/v16.go
index 12cad46d09..9cd4ef4da2 100644
--- a/models/migrations/v16.go
+++ b/models/migrations/v16.go
@@ -26,15 +26,15 @@ type RepoUnit struct {
 
 // Enumerate all the unit types
 const (
-	UnitTypeCode            = iota + 1 // 1 code
-	UnitTypeIssues                     // 2 issues
-	UnitTypePRs                        // 3 PRs
-	UnitTypeCommits                    // 4 Commits
-	UnitTypeReleases                   // 5 Releases
-	UnitTypeWiki                       // 6 Wiki
-	UnitTypeSettings                   // 7 Settings
-	UnitTypeExternalWiki               // 8 ExternalWiki
-	UnitTypeExternalTracker            // 9 ExternalTracker
+	V16UnitTypeCode            = iota + 1 // 1 code
+	V16UnitTypeIssues                     // 2 issues
+	V16UnitTypePRs                        // 3 PRs
+	V16UnitTypeCommits                    // 4 Commits
+	V16UnitTypeReleases                   // 5 Releases
+	V16UnitTypeWiki                       // 6 Wiki
+	V16UnitTypeSettings                   // 7 Settings
+	V16UnitTypeExternalWiki               // 8 ExternalWiki
+	V16UnitTypeExternalTracker            // 9 ExternalTracker
 )
 
 // Repo describes a repository
@@ -79,32 +79,32 @@ func addUnitsToTables(x *xorm.Engine) error {
 
 	for _, repo := range repos {
 		for i := 1; i <= 9; i++ {
-			if (i == UnitTypeWiki || i == UnitTypeExternalWiki) && !repo.EnableWiki {
+			if (i == V16UnitTypeWiki || i == V16UnitTypeExternalWiki) && !repo.EnableWiki {
 				continue
 			}
-			if i == UnitTypeExternalWiki && !repo.EnableExternalWiki {
+			if i == V16UnitTypeExternalWiki && !repo.EnableExternalWiki {
 				continue
 			}
-			if i == UnitTypePRs && !repo.EnablePulls {
+			if i == V16UnitTypePRs && !repo.EnablePulls {
 				continue
 			}
-			if (i == UnitTypeIssues || i == UnitTypeExternalTracker) && !repo.EnableIssues {
+			if (i == V16UnitTypeIssues || i == V16UnitTypeExternalTracker) && !repo.EnableIssues {
 				continue
 			}
-			if i == UnitTypeExternalTracker && !repo.EnableExternalTracker {
+			if i == V16UnitTypeExternalTracker && !repo.EnableExternalTracker {
 				continue
 			}
 
 			var config = make(map[string]string)
 			switch i {
-			case UnitTypeExternalTracker:
+			case V16UnitTypeExternalTracker:
 				config["ExternalTrackerURL"] = repo.ExternalTrackerURL
 				config["ExternalTrackerFormat"] = repo.ExternalTrackerFormat
 				if len(repo.ExternalTrackerStyle) == 0 {
 					repo.ExternalTrackerStyle = markdown.IssueNameStyleNumeric
 				}
 				config["ExternalTrackerStyle"] = repo.ExternalTrackerStyle
-			case UnitTypeExternalWiki:
+			case V16UnitTypeExternalWiki:
 				config["ExternalWikiURL"] = repo.ExternalWikiURL
 			}
 
diff --git a/models/migrations/v38.go b/models/migrations/v38.go
new file mode 100644
index 0000000000..69c346335e
--- /dev/null
+++ b/models/migrations/v38.go
@@ -0,0 +1,56 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"code.gitea.io/gitea/models"
+
+	"github.com/go-xorm/xorm"
+)
+
+func removeCommitsUnitType(x *xorm.Engine) (err error) {
+	// Update team unit types
+	const batchSize = 100
+	for start := 0; ; start += batchSize {
+		teams := make([]*models.Team, 0, batchSize)
+		if err := x.Limit(batchSize, start).Find(&teams); err != nil {
+			return err
+		}
+		if len(teams) == 0 {
+			break
+		}
+		for _, team := range teams {
+			ut := make([]models.UnitType, 0, len(team.UnitTypes))
+			for _, u := range team.UnitTypes {
+				if u < V16UnitTypeCommits {
+					ut = append(ut, u)
+				} else if u > V16UnitTypeSettings {
+					ut = append(ut, u-2)
+				} else if u > V16UnitTypeCommits && u != V16UnitTypeSettings {
+					ut = append(ut, u-1)
+				}
+			}
+			team.UnitTypes = ut
+			if _, err := x.Id(team.ID).Cols("unit_types").Update(team); err != nil {
+				return err
+			}
+		}
+	}
+
+	// Delete commits and settings unit types
+	if _, err = x.In("`type`", []models.UnitType{V16UnitTypeCommits, V16UnitTypeSettings}).Delete(new(RepoUnit)); err != nil {
+		return err
+	}
+	// Fix renumber unit types that where in enumeration after settings unit type
+	if _, err = x.Where("`type` > ?", V16UnitTypeSettings).Decr("type").Decr("index").Update(new(RepoUnit)); err != nil {
+		return err
+	}
+	// Fix renumber unit types that where in enumeration after commits unit type
+	if _, err = x.Where("`type` > ?", V16UnitTypeCommits).Decr("type").Decr("index").Update(new(RepoUnit)); err != nil {
+		return err
+	}
+
+	return nil
+}
diff --git a/models/repo.go b/models/repo.go
index cb6d6d5c3c..a651aae89b 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -377,6 +377,10 @@ func (repo *Repository) getUnitsByUserID(e Engine, userID int64, isAdmin bool) (
 
 	var allTypes = make(map[UnitType]struct{}, len(allRepUnitTypes))
 	for _, team := range teams {
+		// Administrators can not be limited
+		if team.Authorize >= AccessModeAdmin {
+			return nil
+		}
 		for _, unitType := range team.UnitTypes {
 			allTypes[unitType] = struct{}{}
 		}
diff --git a/models/repo_unit.go b/models/repo_unit.go
index f8f01c4398..2256ff7ef6 100644
--- a/models/repo_unit.go
+++ b/models/repo_unit.go
@@ -75,8 +75,8 @@ func (r *RepoUnit) BeforeSet(colName string, val xorm.Cell) {
 	switch colName {
 	case "type":
 		switch UnitType(Cell2Int64(val)) {
-		case UnitTypeCode, UnitTypeIssues, UnitTypePullRequests, UnitTypeCommits, UnitTypeReleases,
-			UnitTypeWiki, UnitTypeSettings:
+		case UnitTypeCode, UnitTypeIssues, UnitTypePullRequests, UnitTypeReleases,
+			UnitTypeWiki:
 			r.Config = new(UnitConfig)
 		case UnitTypeExternalWiki:
 			r.Config = new(ExternalWikiConfig)
@@ -116,11 +116,6 @@ func (r *RepoUnit) PullRequestsConfig() *UnitConfig {
 	return r.Config.(*UnitConfig)
 }
 
-// CommitsConfig returns config for UnitTypeCommits
-func (r *RepoUnit) CommitsConfig() *UnitConfig {
-	return r.Config.(*UnitConfig)
-}
-
 // ReleasesConfig returns config for UnitTypeReleases
 func (r *RepoUnit) ReleasesConfig() *UnitConfig {
 	return r.Config.(*UnitConfig)
diff --git a/models/unit.go b/models/unit.go
index 48ef1620eb..a14edcec0c 100644
--- a/models/unit.go
+++ b/models/unit.go
@@ -12,12 +12,10 @@ const (
 	UnitTypeCode            UnitType = iota + 1 // 1 code
 	UnitTypeIssues                              // 2 issues
 	UnitTypePullRequests                        // 3 PRs
-	UnitTypeCommits                             // 4 Commits
-	UnitTypeReleases                            // 5 Releases
-	UnitTypeWiki                                // 6 Wiki
-	UnitTypeSettings                            // 7 Settings
-	UnitTypeExternalWiki                        // 8 ExternalWiki
-	UnitTypeExternalTracker                     // 9 ExternalTracker
+	UnitTypeReleases                            // 4 Releases
+	UnitTypeWiki                                // 5 Wiki
+	UnitTypeExternalWiki                        // 6 ExternalWiki
+	UnitTypeExternalTracker                     // 7 ExternalTracker
 )
 
 var (
@@ -26,10 +24,8 @@ var (
 		UnitTypeCode,
 		UnitTypeIssues,
 		UnitTypePullRequests,
-		UnitTypeCommits,
 		UnitTypeReleases,
 		UnitTypeWiki,
-		UnitTypeSettings,
 		UnitTypeExternalWiki,
 		UnitTypeExternalTracker,
 	}
@@ -39,22 +35,18 @@ var (
 		UnitTypeCode,
 		UnitTypeIssues,
 		UnitTypePullRequests,
-		UnitTypeCommits,
 		UnitTypeReleases,
 		UnitTypeWiki,
-		UnitTypeSettings,
 	}
 
-	// MustRepoUnits contains the units could be disabled currently
+	// MustRepoUnits contains the units could not be disabled currently
 	MustRepoUnits = []UnitType{
 		UnitTypeCode,
-		UnitTypeCommits,
 		UnitTypeReleases,
-		UnitTypeSettings,
 	}
 )
 
-// Unit is a tab page of one repository
+// Unit is a section of one repository
 type Unit struct {
 	Type    UnitType
 	NameKey string
@@ -65,7 +57,7 @@ type Unit struct {
 
 // CanDisable returns if this unit could be disabled.
 func (u *Unit) CanDisable() bool {
-	return u.Type != UnitTypeSettings
+	return true
 }
 
 // Enumerate all the units
@@ -102,20 +94,12 @@ var (
 		2,
 	}
 
-	UnitCommits = Unit{
-		UnitTypeCommits,
-		"repo.commits",
-		"/commits/master",
-		"repo.commits.desc",
-		3,
-	}
-
 	UnitReleases = Unit{
 		UnitTypeReleases,
 		"repo.releases",
 		"/releases",
 		"repo.releases.desc",
-		4,
+		3,
 	}
 
 	UnitWiki = Unit{
@@ -123,7 +107,7 @@ var (
 		"repo.wiki",
 		"/wiki",
 		"repo.wiki.desc",
-		5,
+		4,
 	}
 
 	UnitExternalWiki = Unit{
@@ -131,15 +115,7 @@ var (
 		"repo.ext_wiki",
 		"/wiki",
 		"repo.ext_wiki.desc",
-		5,
-	}
-
-	UnitSettings = Unit{
-		UnitTypeSettings,
-		"repo.settings",
-		"/settings",
-		"repo.settings.desc",
-		6,
+		4,
 	}
 
 	// Units contains all the units
@@ -148,10 +124,8 @@ var (
 		UnitTypeIssues:          UnitIssues,
 		UnitTypeExternalTracker: UnitExternalTracker,
 		UnitTypePullRequests:    UnitPullRequests,
-		UnitTypeCommits:         UnitCommits,
 		UnitTypeReleases:        UnitReleases,
 		UnitTypeWiki:            UnitWiki,
 		UnitTypeExternalWiki:    UnitExternalWiki,
-		UnitTypeSettings:        UnitSettings,
 	}
 )