From f33e6ae09ec27e898bb8f2cc44d587ea3b8e0dee Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Mon, 17 Jul 2017 05:04:43 +0300 Subject: [PATCH] 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 | 2 +- models/access_test.go | 4 +-- models/fixtures/access.yml | 6 ++++ models/fixtures/repo_unit.yml | 24 +++++++------- models/fixtures/team.yml | 8 ++--- models/migrations/migrations.go | 2 ++ models/migrations/v16.go | 32 +++++++++---------- models/migrations/v38.go | 56 +++++++++++++++++++++++++++++++++ models/repo.go | 4 +++ models/repo_unit.go | 9 ++---- models/unit.go | 46 ++++++--------------------- modules/context/repo.go | 5 +-- options/locale/locale_en-US.ini | 1 + public/css/index.css | 3 ++ public/js/index.js | 13 ++++++++ public/less/_repository.less | 6 ++++ routers/org/teams.go | 47 ++++++++++++++++----------- routers/routes/routes.go | 4 +-- templates/org/team/new.tmpl | 4 +-- templates/repo/header.tmpl | 2 +- 20 files changed, 174 insertions(+), 104 deletions(-) create mode 100644 models/migrations/v38.go diff --git a/integrations/repo_test.go b/integrations/repo_test.go index f5ba4d8d8..8465bbdc5 100644 --- a/integrations/repo_test.go +++ b/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) } diff --git a/models/access_test.go b/models/access_test.go index 29d40d958..59575acb7 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 5ea407610..8722b248d 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 daa348018..02daa4827 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 1b2a0d681..795d5cda6 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 e5b647368..45c410c4d 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 12cad46d0..9cd4ef4da 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 000000000..69c346335 --- /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 cb6d6d5c3..a651aae89 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 f8f01c439..2256ff7ef 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 48ef1620e..a14edcec0 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, } ) diff --git a/modules/context/repo.go b/modules/context/repo.go index d636496f5..3dface7da 100644 --- a/modules/context/repo.go +++ b/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 } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 293657b2f..03d42c214 100644 --- a/options/locale/locale_en-US.ini +++ b/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. diff --git a/public/css/index.css b/public/css/index.css index b37fede0c..8e43a7e16 100644 --- a/public/css/index.css +++ b/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; diff --git a/public/js/index.js b/public/js/index.js index f6da4125b..d79b94b92 100644 --- a/public/js/index.js +++ b/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) { diff --git a/public/less/_repository.less b/public/less/_repository.less index 57b73ea9a..a81d92871 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -36,6 +36,12 @@ } } + .tabs { + .navbar { + justify-content: initial; + } + } + .navbar { display: flex; justify-content: space-between; diff --git a/routers/org/teams.go b/routers/org/teams.go index 10c86bd5c..1ac4bff2e 100644 --- a/routers/org/teams.go +++ b/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 - t.UnitTypes = form.Units + 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 { diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 169f95e28..d085a0910 100644 --- a/routers/routes/routes.go +++ b/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) diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 2f7c5b268..79338394b 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -50,9 +50,8 @@
- {{end}} -
+
+ {{end}}
{{if .PageIsOrgTeamsNew}} diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 1b41a22e1..ed9049130 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -73,7 +73,7 @@ {{end}} - {{if and (.Repository.EnableUnit $.UnitTypeCommits) (not .IsBareRepo)}} + {{if and (.Repository.EnableUnit $.UnitTypeCode) (not .IsBareRepo)}} {{.i18n.Tr "repo.commits"}} {{.CommitsCount}}