From 0b3ea428477b9da33f40252e79aafe85b09526f3 Mon Sep 17 00:00:00 2001 From: David Schneiderbauer Date: Thu, 21 Jun 2018 18:00:13 +0200 Subject: [PATCH] hide issues from org private repos w/o team assignment (#4034) --- integrations/api_repo_test.go | 6 +- models/access_test.go | 16 ++- models/fixtures/repository.yml | 15 ++- models/fixtures/team.yml | 8 +- models/fixtures/team_repo.yml | 10 +- models/fixtures/team_unit.yml | 209 ++++++++++++++++++++++++++++++++ models/fixtures/user.yml | 2 +- models/migrations/migrations.go | 2 + models/migrations/v38.go | 9 +- models/migrations/v69.go | 80 ++++++++++++ models/models.go | 1 + models/notification.go | 10 ++ models/org.go | 17 ++- models/org_team.go | 86 +++++++++++-- models/org_test.go | 12 +- models/repo.go | 18 +-- models/repo_list_test.go | 6 +- models/repo_watch.go | 17 +++ models/user.go | 38 ++++-- models/user_test.go | 22 ++++ routers/org/teams.go | 21 +++- routers/user/home.go | 6 +- routers/user/home_test.go | 4 +- templates/org/team/new.tmpl | 2 +- 24 files changed, 545 insertions(+), 72 deletions(-) create mode 100644 models/fixtures/team_unit.yml create mode 100644 models/migrations/v69.go diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index b766dd5846..12429c88a8 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -67,9 +67,9 @@ func TestAPISearchRepo(t *testing.T) { expectedResults }{ {name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{ - nil: {count: 15}, - user: {count: 15}, - user2: {count: 15}}, + nil: {count: 16}, + user: {count: 16}, + user2: {count: 16}}, }, {name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{ nil: {count: 10}, diff --git a/models/access_test.go b/models/access_test.go index 59575acb7d..46d6f723ea 100644 --- a/models/access_test.go +++ b/models/access_test.go @@ -22,8 +22,12 @@ func TestAccessLevel(t *testing.T) { user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*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) + // A public repository owned by User 2 + repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + assert.False(t, repo1.IsPrivate) + // A private repository owned by Org 3 + repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + assert.True(t, repo2.IsPrivate) level, err := AccessLevel(user1.ID, repo1) assert.NoError(t, err) @@ -47,8 +51,12 @@ func TestHasAccess(t *testing.T) { user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*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) + // A public repository owned by User 2 + repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + assert.False(t, repo1.IsPrivate) + // A private repository owned by Org 3 + repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + assert.True(t, repo2.IsPrivate) for _, accessMode := range accessModes { has, err := HasAccess(user1.ID, repo1, accessMode) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index c7d73fe17d..3238b65ead 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -351,7 +351,7 @@ is_mirror: true num_forks: 1 is_fork: false - + - id: 29 fork_id: 27 @@ -365,7 +365,7 @@ num_closed_pulls: 0 is_mirror: false is_fork: true - + - id: 30 fork_id: 28 @@ -389,3 +389,14 @@ num_forks: 0 num_issues: 0 is_mirror: false + +- + id: 32 + owner_id: 3 + lower_name: repo21 + name: repo21 + is_private: false + num_stars: 0 + num_forks: 0 + num_issues: 0 + is_mirror: false diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 1d242cb5bb..4b4a1d798b 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -4,9 +4,8 @@ lower_name: owners name: Owners authorize: 4 # owner - num_repos: 2 + num_repos: 3 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 2 @@ -16,7 +15,6 @@ authorize: 2 # write num_repos: 1 num_members: 2 - unit_types: '[1,2,3,4,5,6,7]' - id: 3 @@ -26,7 +24,6 @@ authorize: 4 # owner num_repos: 0 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 4 @@ -36,7 +33,6 @@ authorize: 4 # owner num_repos: 0 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 5 @@ -46,7 +42,6 @@ authorize: 4 # owner num_repos: 2 num_members: 2 - unit_types: '[1,2,3,4,5,6,7]' - id: 6 @@ -56,4 +51,3 @@ authorize: 4 # owner num_repos: 2 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' \ No newline at end of file diff --git a/models/fixtures/team_repo.yml b/models/fixtures/team_repo.yml index 9e6d745539..b324e09415 100644 --- a/models/fixtures/team_repo.yml +++ b/models/fixtures/team_repo.yml @@ -33,9 +33,15 @@ org_id: 19 team_id: 6 repo_id: 27 - + - id: 7 org_id: 19 team_id: 6 - repo_id: 28 \ No newline at end of file + repo_id: 28 + +- + id: 8 + org_id: 3 + team_id: 1 + repo_id: 32 diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml new file mode 100644 index 0000000000..ad5466a5c1 --- /dev/null +++ b/models/fixtures/team_unit.yml @@ -0,0 +1,209 @@ +- + id: 1 + team_id: 1 + type: 1 + +- + id: 2 + team_id: 1 + type: 2 + +- + id: 3 + team_id: 1 + type: 3 + +- + id: 4 + team_id: 1 + type: 4 + +- + id: 5 + team_id: 1 + type: 5 + +- + id: 6 + team_id: 1 + type: 6 + +- + id: 7 + team_id: 1 + type: 7 + +- + id: 8 + team_id: 2 + type: 1 + +- + id: 9 + team_id: 2 + type: 2 + +- + id: 10 + team_id: 2 + type: 3 + +- + id: 11 + team_id: 2 + type: 4 + +- + id: 12 + team_id: 2 + type: 5 + +- + id: 13 + team_id: 2 + type: 6 + +- + id: 14 + team_id: 2 + type: 7 + +- + id: 15 + team_id: 3 + type: 1 + +- + id: 16 + team_id: 3 + type: 2 + +- + id: 17 + team_id: 3 + type: 3 + +- + id: 18 + team_id: 3 + type: 4 + +- + id: 19 + team_id: 3 + type: 5 + +- + id: 20 + team_id: 3 + type: 6 + +- + id: 21 + team_id: 3 + type: 7 + +- + id: 22 + team_id: 4 + type: 1 + +- + id: 23 + team_id: 4 + type: 2 + +- + id: 24 + team_id: 4 + type: 3 + +- + id: 25 + team_id: 4 + type: 4 + +- + id: 26 + team_id: 4 + type: 5 + +- + id: 27 + team_id: 4 + type: 6 + +- + id: 28 + team_id: 4 + type: 7 + +- + id: 29 + team_id: 5 + type: 1 + +- + id: 30 + team_id: 5 + type: 2 + +- + id: 31 + team_id: 5 + type: 3 + +- + id: 32 + team_id: 5 + type: 4 + +- + id: 33 + team_id: 5 + type: 5 + +- + id: 34 + team_id: 5 + type: 6 + +- + id: 35 + team_id: 5 + type: 7 + +- + id: 36 + team_id: 6 + type: 1 + +- + id: 37 + team_id: 6 + type: 2 + +- + id: 38 + team_id: 6 + type: 3 + +- + id: 39 + team_id: 6 + type: 4 + +- + id: 40 + team_id: 6 + type: 5 + +- + id: 41 + team_id: 6 + type: 6 + +- + id: 42 + team_id: 6 + type: 7 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 7ad48f7fbe..a2e3b88d79 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -45,7 +45,7 @@ is_admin: false avatar: avatar3 avatar_email: user3@example.com - num_repos: 2 + num_repos: 3 num_members: 2 num_teams: 2 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7732e17094..cc262d8102 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -190,6 +190,8 @@ var migrations = []Migration{ NewMigration("remove stale watches", removeStaleWatches), // v68 -> V69 NewMigration("Reformat and remove incorrect topics", reformatAndRemoveIncorrectTopics), + // v69 -> v70 + NewMigration("move team units to team_unit table", moveTeamUnitsToTeamUnitTable), } // Migrate database to current version diff --git a/models/migrations/v38.go b/models/migrations/v38.go index 6f66484b05..eb90f9fbff 100644 --- a/models/migrations/v38.go +++ b/models/migrations/v38.go @@ -25,10 +25,15 @@ func removeCommitsUnitType(x *xorm.Engine) (err error) { Created time.Time `xorm:"-"` } + type Team struct { + ID int64 + UnitTypes []int `xorm:"json"` + } + // Update team unit types const batchSize = 100 for start := 0; ; start += batchSize { - teams := make([]*models.Team, 0, batchSize) + teams := make([]*Team, 0, batchSize) if err := x.Limit(batchSize, start).Find(&teams); err != nil { return err } @@ -36,7 +41,7 @@ func removeCommitsUnitType(x *xorm.Engine) (err error) { break } for _, team := range teams { - ut := make([]models.UnitType, 0, len(team.UnitTypes)) + ut := make([]int, 0, len(team.UnitTypes)) for _, u := range team.UnitTypes { if u < V16UnitTypeCommits { ut = append(ut, u) diff --git a/models/migrations/v69.go b/models/migrations/v69.go new file mode 100644 index 0000000000..8d964688ae --- /dev/null +++ b/models/migrations/v69.go @@ -0,0 +1,80 @@ +// Copyright 2018 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 ( + "fmt" + + "github.com/go-xorm/xorm" +) + +func moveTeamUnitsToTeamUnitTable(x *xorm.Engine) error { + // Team see models/team.go + type Team struct { + ID int64 + OrgID int64 + UnitTypes []int `xorm:"json"` + } + + // TeamUnit see models/org_team.go + type TeamUnit struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + TeamID int64 `xorm:"UNIQUE(s)"` + Type int `xorm:"UNIQUE(s)"` + } + + if err := x.Sync2(new(TeamUnit)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + // Update team unit types + const batchSize = 100 + for start := 0; ; start += batchSize { + teams := make([]*Team, 0, batchSize) + if err := x.Limit(batchSize, start).Find(&teams); err != nil { + return err + } + if len(teams) == 0 { + break + } + + for _, team := range teams { + var unitTypes []int + if len(team.UnitTypes) == 0 { + unitTypes = allUnitTypes + } else { + unitTypes = team.UnitTypes + } + + // insert units for team + var units = make([]TeamUnit, 0, len(unitTypes)) + for _, tp := range unitTypes { + units = append(units, TeamUnit{ + OrgID: team.OrgID, + TeamID: team.ID, + Type: tp, + }) + } + + if _, err := sess.Insert(&units); err != nil { + return fmt.Errorf("Insert team units: %v", err) + } + + } + } + + if err := dropTableColumns(sess, "team", "unit_types"); err != nil { + return err + } + return sess.Commit() +} diff --git a/models/models.go b/models/models.go index 5743f1862d..aaf1370fd4 100644 --- a/models/models.go +++ b/models/models.go @@ -122,6 +122,7 @@ func init() { new(Reaction), new(IssueAssignees), new(U2FRegistration), + new(TeamUnit), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/notification.go b/models/notification.go index c8376a857b..8c36c0c5c9 100644 --- a/models/notification.go +++ b/models/notification.go @@ -119,7 +119,17 @@ func createOrUpdateIssueNotifications(e Engine, issue *Issue, notificationAuthor } } + issue.loadRepo(e) + for _, watch := range watches { + issue.Repo.Units = nil + if issue.IsPull && !issue.Repo.CheckUnitUser(watch.UserID, false, UnitTypePullRequests) { + continue + } + if !issue.IsPull && !issue.Repo.CheckUnitUser(watch.UserID, false, UnitTypeIssues) { + continue + } + if err := notifyUser(watch.UserID); err != nil { return err } diff --git a/models/org.go b/models/org.go index ed0d583067..23f6c58bf6 100644 --- a/models/org.go +++ b/models/org.go @@ -154,12 +154,26 @@ func CreateOrganization(org, owner *User) (err error) { Name: ownerTeamName, Authorize: AccessModeOwner, NumMembers: 1, - UnitTypes: allRepUnitTypes, } if _, err = sess.Insert(t); err != nil { return fmt.Errorf("insert owner team: %v", err) } + // insert units for team + var units = make([]TeamUnit, 0, len(allRepUnitTypes)) + for _, tp := range allRepUnitTypes { + units = append(units, TeamUnit{ + OrgID: org.ID, + TeamID: t.ID, + Type: tp, + }) + } + + if _, err = sess.Insert(&units); err != nil { + sess.Rollback() + return err + } + if _, err = sess.Insert(&TeamUser{ UID: owner.ID, OrgID: org.ID, @@ -238,6 +252,7 @@ func deleteOrg(e *xorm.Session, u *User) error { &Team{OrgID: u.ID}, &OrgUser{OrgID: u.ID}, &TeamUser{OrgID: u.ID}, + &TeamUnit{OrgID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/org_team.go b/models/org_team.go index 5ea6e76cd9..3b37f936ff 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -1,3 +1,4 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. // Copyright 2016 The Gogs Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -10,7 +11,6 @@ import ( "strings" "code.gitea.io/gitea/modules/log" - "github.com/go-xorm/xorm" ) @@ -28,15 +28,16 @@ type Team struct { Members []*User `xorm:"-"` NumRepos int NumMembers int - UnitTypes []UnitType `xorm:"json"` + Units []*TeamUnit `xorm:"-"` } -// GetUnitTypes returns unit types the team owned, empty means all the unit types -func (t *Team) GetUnitTypes() []UnitType { - if len(t.UnitTypes) == 0 { - return allRepUnitTypes +func (t *Team) getUnits(e Engine) (err error) { + if t.Units != nil { + return nil } - return t.UnitTypes + + t.Units, err = getUnitsByTeamID(e, t.ID) + return err } // HasWriteAccess returns true if team has at least write level access mode. @@ -214,11 +215,12 @@ func (t *Team) RemoveRepository(repoID int64) error { // UnitEnabled returns if the team has the given unit type enabled func (t *Team) UnitEnabled(tp UnitType) bool { - if len(t.UnitTypes) == 0 { - return true + if err := t.getUnits(x); err != nil { + log.Warn("Error loading repository (ID: %d) units: %s", t.ID, err.Error()) } - for _, u := range t.UnitTypes { - if u == tp { + + for _, unit := range t.Units { + if unit.Type == tp { return true } } @@ -275,6 +277,17 @@ func NewTeam(t *Team) (err error) { return err } + // insert units for team + if len(t.Units) > 0 { + for _, unit := range t.Units { + unit.TeamID = t.ID + } + if _, err = sess.Insert(&t.Units); err != nil { + sess.Rollback() + return err + } + } + // Update organization number of teams. if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { sess.Rollback() @@ -424,6 +437,13 @@ func DeleteTeam(t *Team) error { return err } + // Delete team-unit. + if _, err := sess. + Where("team_id=?", t.ID). + Delete(new(TeamUnit)); err != nil { + return err + } + // Delete team. if _, err := sess.ID(t.ID).Delete(new(Team)); err != nil { return err @@ -695,3 +715,47 @@ func GetTeamsWithAccessToRepo(orgID, repoID int64, mode AccessMode) ([]*Team, er And("team_repo.repo_id = ?", repoID). Find(&teams) } + +// ___________ ____ ___ .__ __ +// \__ ___/___ _____ _____ | | \____ |__|/ |_ +// | |_/ __ \\__ \ / \| | / \| \ __\ +// | |\ ___/ / __ \| Y Y \ | / | \ || | +// |____| \___ >____ /__|_| /______/|___| /__||__| +// \/ \/ \/ \/ + +// TeamUnit describes all units of a repository +type TeamUnit struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + TeamID int64 `xorm:"UNIQUE(s)"` + Type UnitType `xorm:"UNIQUE(s)"` +} + +// Unit returns Unit +func (t *TeamUnit) Unit() Unit { + return Units[t.Type] +} + +func getUnitsByTeamID(e Engine, teamID int64) (units []*TeamUnit, err error) { + return units, e.Where("team_id = ?", teamID).Find(&units) +} + +// UpdateTeamUnits updates a teams's units +func UpdateTeamUnits(team *Team, units []TeamUnit) (err error) { + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + if _, err = sess.Where("team_id = ?", team.ID).Delete(new(TeamUnit)); err != nil { + return err + } + + if _, err = sess.Insert(units); err != nil { + sess.Rollback() + return err + } + + return sess.Commit() +} diff --git a/models/org_test.go b/models/org_test.go index 42ab4a2a45..c54e7a93bf 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -489,8 +489,8 @@ func TestAccessibleReposEnv_CountRepos(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, expectedCount, count) } - testSuccess(2, 2) - testSuccess(4, 1) + testSuccess(2, 3) + testSuccess(4, 2) } func TestAccessibleReposEnv_RepoIDs(t *testing.T) { @@ -503,8 +503,8 @@ func TestAccessibleReposEnv_RepoIDs(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expectedRepoIDs, repoIDs) } - testSuccess(2, 1, 100, []int64{3, 5}) - testSuccess(4, 0, 100, []int64{3}) + testSuccess(2, 1, 100, []int64{3, 5, 32}) + testSuccess(4, 0, 100, []int64{3, 32}) } func TestAccessibleReposEnv_Repos(t *testing.T) { @@ -522,8 +522,8 @@ func TestAccessibleReposEnv_Repos(t *testing.T) { } assert.Equal(t, expectedRepos, repos) } - testSuccess(2, []int64{3, 5}) - testSuccess(4, []int64{3}) + testSuccess(2, []int64{3, 5, 32}) + testSuccess(4, []int64{3, 32}) } func TestAccessibleReposEnv_MirrorRepos(t *testing.T) { diff --git a/models/repo.go b/models/repo.go index 7f2be502a4..d1cc290c44 100644 --- a/models/repo.go +++ b/models/repo.go @@ -365,22 +365,14 @@ func (repo *Repository) getUnitsByUserID(e Engine, userID int64, isAdmin bool) ( return err } - 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{}{} - } - } - // unique var newRepoUnits = make([]*RepoUnit, 0, len(repo.Units)) for _, u := range repo.Units { - if _, ok := allTypes[u.Type]; ok { - newRepoUnits = append(newRepoUnits, u) + for _, team := range teams { + if team.UnitEnabled(u.Type) { + newRepoUnits = append(newRepoUnits, u) + break + } } } diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 3bccb1aebe..164bc19bf0 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -147,10 +147,10 @@ func TestSearchRepositoryByName(t *testing.T) { count: 14}, {name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, AllPublic: true}, - count: 15}, + count: 16}, {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true}, - count: 19}, + count: 20}, {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", opts: &SearchRepoOptions{Keyword: "test", Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true}, count: 13}, @@ -159,7 +159,7 @@ func TestSearchRepositoryByName(t *testing.T) { count: 11}, {name: "AllPublic/PublicRepositoriesOfOrganization", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse}, - count: 15}, + count: 16}, } for _, testCase := range testCases { diff --git a/models/repo_watch.go b/models/repo_watch.go index fb89a55a11..8019027c1e 100644 --- a/models/repo_watch.go +++ b/models/repo_watch.go @@ -109,6 +109,23 @@ func notifyWatchers(e Engine, act *Action) error { act.ID = 0 act.UserID = watches[i].UserID + act.Repo.Units = nil + + switch act.OpType { + case ActionCommitRepo, ActionPushTag, ActionDeleteTag, ActionDeleteBranch: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeCode) { + continue + } + case ActionCreateIssue, ActionCommentIssue, ActionCloseIssue, ActionReopenIssue: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeIssues) { + continue + } + case ActionCreatePullRequest, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypePullRequests) { + continue + } + } + if _, err = e.InsertOne(act); err != nil { return fmt.Errorf("insert new action: %v", err) } diff --git a/models/user.go b/models/user.go index 1497eef44d..653e994263 100644 --- a/models/user.go +++ b/models/user.go @@ -546,28 +546,46 @@ func (u *User) GetRepositories(page, pageSize int) (err error) { return err } -// GetRepositoryIDs returns repositories IDs where user owned -func (u *User) GetRepositoryIDs() ([]int64, error) { +// GetRepositoryIDs returns repositories IDs where user owned and has unittypes +func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) { var ids []int64 - return ids, x.Table("repository").Cols("id").Where("owner_id = ?", u.ID).Find(&ids) + + sess := x.Table("repository").Cols("repository.id") + + if len(units) > 0 { + sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id") + sess = sess.In("repo_unit.type", units) + } + + return ids, sess.Where("owner_id = ?", u.ID).Find(&ids) } -// GetOrgRepositoryIDs returns repositories IDs where user's team owned -func (u *User) GetOrgRepositoryIDs() ([]int64, error) { +// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes +func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) { var ids []int64 - return ids, x.Table("repository"). + + sess := x.Table("repository"). Cols("repository.id"). - Join("INNER", "team_user", "repository.owner_id = team_user.org_id AND team_user.uid = ?", u.ID). + Join("INNER", "team_user", "repository.owner_id = team_user.org_id"). + Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true) + + if len(units) > 0 { + sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id") + sess = sess.In("team_unit.type", units) + } + + return ids, sess. + Where("team_user.uid = ?", u.ID). GroupBy("repository.id").Find(&ids) } // GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations -func (u *User) GetAccessRepoIDs() ([]int64, error) { - ids, err := u.GetRepositoryIDs() +func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) { + ids, err := u.GetRepositoryIDs(units...) if err != nil { return nil, err } - ids2, err := u.GetOrgRepositoryIDs() + ids2, err := u.GetOrgRepositoryIDs(units...) if err != nil { return nil, err } diff --git a/models/user_test.go b/models/user_test.go index 4fd0bc0fad..20de1a64be 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -159,3 +159,25 @@ func BenchmarkHashPassword(b *testing.B) { u.HashPassword(pass) } } + +func TestGetOrgRepositoryIDs(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User) + user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) + + accessibleRepos, err := user2.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization + assert.Equal(t, []int64{3, 5, 32}, accessibleRepos) + + accessibleRepos, err = user4.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 4's team has access to private repo 3, repo 32 is a public repo of the organization + assert.Equal(t, []int64{3, 32}, accessibleRepos) + + accessibleRepos, err = user5.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 5's team has no access to any repo + assert.Len(t, accessibleRepos, 0) +} diff --git a/routers/org/teams.go b/routers/org/teams.go index d894c86614..87bfb8596a 100644 --- a/routers/org/teams.go +++ b/routers/org/teams.go @@ -182,7 +182,14 @@ func NewTeamPost(ctx *context.Context, form auth.CreateTeamForm) { Authorize: models.ParseAccessMode(form.Permission), } if t.Authorize < models.AccessModeAdmin { - t.UnitTypes = form.Units + var units = make([]*models.TeamUnit, 0, len(form.Units)) + for _, tp := range form.Units { + units = append(units, &models.TeamUnit{ + OrgID: ctx.Org.Organization.ID, + Type: tp, + }) + } + t.Units = units } ctx.Data["Team"] = t @@ -264,9 +271,17 @@ func EditTeamPost(ctx *context.Context, form auth.CreateTeamForm) { } t.Description = form.Description if t.Authorize < models.AccessModeAdmin { - t.UnitTypes = form.Units + var units = make([]models.TeamUnit, 0, len(form.Units)) + for _, tp := range form.Units { + units = append(units, models.TeamUnit{ + OrgID: t.OrgID, + TeamID: t.ID, + Type: tp, + }) + } + models.UpdateTeamUnits(t, units) } else { - t.UnitTypes = nil + models.UpdateTeamUnits(t, nil) } if ctx.HasError() { diff --git a/routers/user/home.go b/routers/user/home.go index 2a193bbdef..0c84b2498e 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -203,7 +203,11 @@ func Issues(ctx *context.Context) { return } } else { - userRepoIDs, err = ctxUser.GetAccessRepoIDs() + unitType := models.UnitTypeIssues + if isPullList { + unitType = models.UnitTypePullRequests + } + userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType) if err != nil { ctx.ServerError("ctxUser.GetAccessRepoIDs", err) return diff --git a/routers/user/home_test.go b/routers/user/home_test.go index a9b146b762..8a3d9b9f51 100644 --- a/routers/user/home_test.go +++ b/routers/user/home_test.go @@ -26,8 +26,8 @@ func TestIssues(t *testing.T) { Issues(ctx) assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) - assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"]) + assert.EqualValues(t, map[int64]int64{1: 1}, ctx.Data["Counts"]) assert.EqualValues(t, true, ctx.Data["IsShowClosed"]) assert.Len(t, ctx.Data["Issues"], 1) - assert.Len(t, ctx.Data["Repos"], 2) + assert.Len(t, ctx.Data["Repos"], 1) } diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index ec1a3dd72e..12cdd697c3 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -57,7 +57,7 @@ {{range $t, $unit := $.Units}}
- + {{$.i18n.Tr $unit.DescKey}}