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

integrations/repo_test.go

@@ -39,7 +39,7 @@ func TestViewRepo3(t *testing.T) {
 	prepareTestEnv(t)
 
 	req := NewRequest(t, "GET", "/user3/repo3")
-	session := loginUser(t, "user3")
+	session := loginUser(t, "user4")
 	session.MakeRequest(t, req, http.StatusOK)
 }
 

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)
 

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

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

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]'
+  unit_types: '[1,2,3,4,5,6,7]'

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

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
 			}
 

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
+}

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{}{}
 		}

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)

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,
 	}
 )

modules/context/repo.go

@@ -348,6 +348,9 @@ func RepoAssignment() macaron.Handler {
 					ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
 				}
 			}
+
+			// Reset repo units as otherwise user specific units wont be loaded later
+			ctx.Repo.Repository.Units = nil
 		}
 		ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest
 
@@ -549,10 +552,8 @@ func UnitTypes() macaron.Handler {
 		ctx.Data["UnitTypeCode"] = models.UnitTypeCode
 		ctx.Data["UnitTypeIssues"] = models.UnitTypeIssues
 		ctx.Data["UnitTypePullRequests"] = models.UnitTypePullRequests
-		ctx.Data["UnitTypeCommits"] = models.UnitTypeCommits
 		ctx.Data["UnitTypeReleases"] = models.UnitTypeReleases
 		ctx.Data["UnitTypeWiki"] = models.UnitTypeWiki
-		ctx.Data["UnitTypeSettings"] = models.UnitTypeSettings
 		ctx.Data["UnitTypeExternalWiki"] = models.UnitTypeExternalWiki
 		ctx.Data["UnitTypeExternalTracker"] = models.UnitTypeExternalTracker
 	}

options/locale/locale_en-US.ini

@@ -251,6 +251,7 @@ username_been_taken = Username already taken.
 repo_name_been_taken = Repository name already used.
 org_name_been_taken = Organization name already taken.
 team_name_been_taken = Team name already taken.
+team_no_units_error = Team must have at least one unit enabled.
 email_been_used = Email already used.
 openid_been_used = OpenID address '%s' already used.
 username_password_incorrect = Incorrect username or password.

public/css/index.css

@@ -1202,6 +1202,9 @@ footer .ui.language .menu {
   margin-top: -1px;
   font-size: 15px;
 }
+.repository .tabs .navbar {
+  justify-content: initial;
+}
 .repository .navbar {
   display: flex;
   justify-content: space-between;

public/js/index.js

@@ -657,6 +657,18 @@ function initRepositoryCollaboration() {
     });
 }
 
+function initTeamSettings() {
+    // Change team access mode
+    $('.organization.new.team input[name=permission]').change(function () {
+        var val = $('input[name=permission]:checked', '.organization.new.team').val()
+        if (val === 'admin') {
+            $('.organization.new.team .team-units').hide();
+        } else {
+            $('.organization.new.team .team-units').show();
+        }
+    });
+}
+
 function initWikiForm() {
     var $editArea = $('.repository.wiki textarea#edit_area');
     if ($editArea.length > 0) {
@@ -1557,6 +1569,7 @@ $(document).ready(function () {
     initAdmin();
     initCodeView();
     initDashboardSearch();
+    initTeamSettings();
 
     // Repo clone url.
     if ($('#repo-clone-url').length > 0) {

public/less/_repository.less

@@ -36,6 +36,12 @@
 		}
 	}
 
+	.tabs {
+		.navbar {
+			justify-content: initial;
+		}
+	}
+
 	.navbar {
 		display: flex;
 		justify-content: space-between;

routers/org/teams.go

@@ -171,14 +171,18 @@ func NewTeamPost(ctx *context.Context, form auth.CreateTeamForm) {
 	ctx.Data["Title"] = ctx.Org.Organization.FullName
 	ctx.Data["PageIsOrgTeams"] = true
 	ctx.Data["PageIsOrgTeamsNew"] = true
+	ctx.Data["Units"] = models.Units
 
 	t := &models.Team{
 		OrgID:       ctx.Org.Organization.ID,
 		Name:        form.TeamName,
 		Description: form.Description,
 		Authorize:   models.ParseAccessMode(form.Permission),
-		UnitTypes:   form.Units,
 	}
+	if t.Authorize < models.AccessModeAdmin {
+		t.UnitTypes = form.Units
+	}
+
 	ctx.Data["Team"] = t
 
 	if ctx.HasError() {
@@ -186,6 +190,11 @@ func NewTeamPost(ctx *context.Context, form auth.CreateTeamForm) {
 		return
 	}
 
+	if t.Authorize < models.AccessModeAdmin && len(form.Units) == 0 {
+		ctx.RenderWithErr(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form)
+		return
+	}
+
 	if err := models.NewTeam(t); err != nil {
 		ctx.Data["Err_TeamName"] = true
 		switch {
@@ -238,27 +247,12 @@ func EditTeamPost(ctx *context.Context, form auth.CreateTeamForm) {
 	ctx.Data["Title"] = ctx.Org.Organization.FullName
 	ctx.Data["PageIsOrgTeams"] = true
 	ctx.Data["Team"] = t
-
-	if ctx.HasError() {
-		ctx.HTML(200, tplTeamNew)
-		return
-	}
+	ctx.Data["Units"] = models.Units
 
 	isAuthChanged := false
 	if !t.IsOwnerTeam() {
 		// Validate permission level.
-		var auth models.AccessMode
-		switch form.Permission {
-		case "read":
-			auth = models.AccessModeRead
-		case "write":
-			auth = models.AccessModeWrite
-		case "admin":
-			auth = models.AccessModeAdmin
-		default:
-			ctx.Error(401)
-			return
-		}
+		auth := models.ParseAccessMode(form.Permission)
 
 		t.Name = form.TeamName
 		if t.Authorize != auth {
@@ -267,7 +261,22 @@ func EditTeamPost(ctx *context.Context, form auth.CreateTeamForm) {
 		}
 	}
 	t.Description = form.Description
+	if t.Authorize < models.AccessModeAdmin {
 		t.UnitTypes = form.Units
+	} else {
+		t.UnitTypes = nil
+	}
+
+	if ctx.HasError() {
+		ctx.HTML(200, tplTeamNew)
+		return
+	}
+
+	if t.Authorize < models.AccessModeAdmin && len(form.Units) == 0 {
+		ctx.RenderWithErr(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form)
+		return
+	}
+
 	if err := models.UpdateTeam(t, isAuthChanged); err != nil {
 		ctx.Data["Err_TeamName"] = true
 		switch {

routers/routes/routes.go

@@ -450,8 +450,8 @@ func RegisterRoutes(m *macaron.Macaron) {
 
 		}, func(ctx *context.Context) {
 			ctx.Data["PageIsSettings"] = true
-		}, context.UnitTypes(), context.LoadRepoUnits(), context.CheckUnit(models.UnitTypeSettings))
-	}, reqSignIn, context.RepoAssignment(), reqRepoAdmin, context.RepoRef())
+		})
+	}, reqSignIn, context.RepoAssignment(), reqRepoAdmin, context.UnitTypes(), context.LoadRepoUnits(), context.RepoRef())
 
 	m.Get("/:username/:reponame/action/:action", reqSignIn, context.RepoAssignment(), repo.Action)
 

templates/org/team/new.tmpl

@@ -50,9 +50,8 @@
 							</div>
 						</div>
 						<div class="ui divider"></div>
-					{{end}}
 
-					<div class="required grouped field">
+						<div class="team-units required grouped field"{{if eq .Team.Authorize 3}} style="display: none"{{end}}>
 							<label>{{.i18n.Tr "org.team_unit_desc"}}</label>
 							<br>
 							{{range $t, $unit := $.Units}}
@@ -66,6 +65,7 @@
 							{{end}}
 						</div>
 						<div class="ui divider"></div>
+					{{end}}
 
 					<div class="field">
 						{{if .PageIsOrgTeamsNew}}

templates/repo/header.tmpl

@@ -73,7 +73,7 @@
 				</a>
 			{{end}}
 
-			{{if and (.Repository.EnableUnit $.UnitTypeCommits) (not .IsBareRepo)}}
+			{{if and (.Repository.EnableUnit $.UnitTypeCode) (not .IsBareRepo)}}
 			<a class="{{if (or (.PageIsCommits) (.PageIsDiff))}}active{{end}} item" href="{{.RepoLink}}/commits/{{EscapePound .BranchName}}">
 				<i class="octicon octicon-history"></i> {{.i18n.Tr "repo.commits"}} <span class="ui {{if not .CommitsCount}}gray{{else}}blue{{end}} small label">{{.CommitsCount}}</span>
 			</a>