aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLauris BH2019-10-08 19:12:41 +0300
committerAntoine GIRARD2019-10-08 18:12:41 +0200
commit05f6566786d6b262c5050130cf8577739c8906bb (patch)
tree4572532e82a0653218d771ff3cd68efcccf3a5d3
parent0ea4b786cb99054a381f3005f11f4cf413214617 (diff)
Revert "[Backport] Fix issues/pr list broken when there are man… (#8425)
This reverts commit 0ea4b786cb99054a381f3005f11f4cf413214617.
-rw-r--r--models/issue.go54
-rw-r--r--models/issue_test.go11
-rw-r--r--models/user.go61
-rw-r--r--models/user_test.go22
-rw-r--r--routers/user/home.go140
5 files changed, 178 insertions, 110 deletions
diff --git a/models/issue.go b/models/issue.go
index 22d05a161..33e13bb18 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -1291,19 +1291,18 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
// IssuesOptions represents options of an issue.
type IssuesOptions struct {
- RepoIDs []int64 // include all repos if empty
- RepoSubQuery *builder.Builder
- AssigneeID int64
- PosterID int64
- MentionedID int64
- MilestoneID int64
- Page int
- PageSize int
- IsClosed util.OptionalBool
- IsPull util.OptionalBool
- LabelIDs []int64
- SortType string
- IssueIDs []int64
+ RepoIDs []int64 // include all repos if empty
+ AssigneeID int64
+ PosterID int64
+ MentionedID int64
+ MilestoneID int64
+ Page int
+ PageSize int
+ IsClosed util.OptionalBool
+ IsPull util.OptionalBool
+ LabelIDs []int64
+ SortType string
+ IssueIDs []int64
}
// sortIssuesSession sort an issues-related session based on the provided
@@ -1346,9 +1345,7 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
sess.In("issue.id", opts.IssueIDs)
}
- if opts.RepoSubQuery != nil {
- sess.In("issue.repo_id", opts.RepoSubQuery)
- } else if len(opts.RepoIDs) > 0 {
+ if len(opts.RepoIDs) > 0 {
// In case repository IDs are provided but actually no repository has issue.
sess.In("issue.repo_id", opts.RepoIDs)
}
@@ -1615,12 +1612,12 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
type UserIssueStatsOptions struct {
- UserID int64
- RepoID int64
- RepoSubQuery *builder.Builder
- FilterMode int
- IsPull bool
- IsClosed bool
+ UserID int64
+ RepoID int64
+ UserRepoIDs []int64
+ FilterMode int
+ IsPull bool
+ IsClosed bool
}
// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1634,23 +1631,16 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
}
- var repoCond = builder.NewCond()
- if opts.RepoSubQuery != nil {
- repoCond = builder.In("issue.repo_id", opts.RepoSubQuery)
- } else {
- repoCond = builder.Expr("0=1")
- }
-
switch opts.FilterMode {
case FilterModeAll:
stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
- And(repoCond).
+ And(builder.In("issue.repo_id", opts.UserRepoIDs)).
Count(new(Issue))
if err != nil {
return nil, err
}
stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
- And(repoCond).
+ And(builder.In("issue.repo_id", opts.UserRepoIDs)).
Count(new(Issue))
if err != nil {
return nil, err
@@ -1702,7 +1692,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
}
stats.YourRepositoriesCount, err = x.Where(cond).
- And(repoCond).
+ And(builder.In("issue.repo_id", opts.UserRepoIDs)).
Count(new(Issue))
if err != nil {
return nil, err
diff --git a/models/issue_test.go b/models/issue_test.go
index 4edc241c8..1a7e45ae0 100644
--- a/models/issue_test.go
+++ b/models/issue_test.go
@@ -10,7 +10,6 @@ import (
"time"
"github.com/stretchr/testify/assert"
- "xorm.io/builder"
)
func TestIssue_ReplaceLabels(t *testing.T) {
@@ -267,12 +266,10 @@ func TestGetUserIssueStats(t *testing.T) {
},
{
UserIssueStatsOptions{
- UserID: 2,
- RepoSubQuery: builder.Select("repository.id").
- From("repository").
- Where(builder.In("repository.id", []int64{1, 2})),
- FilterMode: FilterModeAll,
- IsClosed: true,
+ UserID: 2,
+ UserRepoIDs: []int64{1, 2},
+ FilterMode: FilterModeAll,
+ IsClosed: true,
},
IssueStats{
YourRepositoriesCount: 2,
diff --git a/models/user.go b/models/user.go
index 4286163f5..e5e30ea9e 100644
--- a/models/user.go
+++ b/models/user.go
@@ -561,35 +561,50 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
return err
}
-// UnitRepositoriesSubQuery returns repositories query builder according units
-func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder {
- b := builder.Select("repository.id").From("repository")
+// GetRepositoryIDs returns repositories IDs where user owned and has unittypes
+func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
+ var ids []int64
+
+ sess := x.Table("repository").Cols("repository.id")
if len(units) > 0 {
- b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id").
- And(builder.In("repo_unit.type", units)),
- )
+ sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
+ sess = sess.In("repo_unit.type", units)
}
- return b.Where(builder.Eq{"repository.owner_id": u.ID})
+
+ return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
}
-// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units
-func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder {
- b := builder.
- Select("team_repo.repo_id").
- From("team_repo").
- Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And(
- builder.Expr("team_user.team_id = team_repo.team_id"),
- ))
+// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
+func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
+ var ids []int64
+
+ sess := x.Table("repository").
+ Cols("repository.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 {
- b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And(
- builder.Expr("team_unit.team_id = team_repo.team_id").And(
- builder.In("`type`", units),
- ),
- ))
- }
- return b.Where(builder.Eq{"team_repo.org_id": u.ID}).
- GroupBy("team_repo.repo_id")
+ 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(units ...UnitType) ([]int64, error) {
+ ids, err := u.GetRepositoryIDs(units...)
+ if err != nil {
+ return nil, err
+ }
+ ids2, err := u.GetOrgRepositoryIDs(units...)
+ if err != nil {
+ return nil, err
+ }
+ return append(ids, ids2...), nil
}
// GetMirrorRepositories returns mirror repositories that user owns, including private repositories.
diff --git a/models/user_test.go b/models/user_test.go
index 3ba908e95..6af9752c9 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -178,6 +178,28 @@ func BenchmarkHashPassword(b *testing.B) {
}
}
+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, 23, 24, 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)
+}
+
func TestNewGitSig(t *testing.T) {
users := make([]*User, 0, 20)
sess := x.NewSession()
diff --git a/routers/user/home.go b/routers/user/home.go
index 2ee0e0d1d..9ccd5bdb2 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -14,9 +14,11 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
+ "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
+ "github.com/Unknwon/com"
"github.com/keybase/go-crypto/openpgp"
"github.com/keybase/go-crypto/openpgp/armor"
)
@@ -150,24 +152,6 @@ func Dashboard(ctx *context.Context) {
// Issues render the user issues page
func Issues(ctx *context.Context) {
isPullList := ctx.Params(":type") == "pulls"
- repoID := ctx.QueryInt64("repo")
- if repoID > 0 {
- repo, err := models.GetRepositoryByID(repoID)
- if err != nil {
- ctx.ServerError("GetRepositoryByID", err)
- return
- }
- perm, err := models.GetUserRepoPermission(repo, ctx.User)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return
- }
- if !perm.CanReadIssuesOrPulls(isPullList) {
- ctx.NotFound("Repository does not exist or you have no permission", nil)
- return
- }
- }
-
if isPullList {
ctx.Data["Title"] = ctx.Tr("pull_requests")
ctx.Data["PageIsPulls"] = true
@@ -208,32 +192,58 @@ func Issues(ctx *context.Context) {
page = 1
}
- var (
- isShowClosed = ctx.Query("state") == "closed"
- err error
- opts = &models.IssuesOptions{
- IsClosed: util.OptionalBoolOf(isShowClosed),
- IsPull: util.OptionalBoolOf(isPullList),
- SortType: sortType,
- }
- )
+ repoID := ctx.QueryInt64("repo")
+ isShowClosed := ctx.Query("state") == "closed"
// Get repositories.
- if repoID > 0 {
- opts.RepoIDs = []int64{repoID}
+ var err error
+ var userRepoIDs []int64
+ if ctxUser.IsOrganization() {
+ env, err := ctxUser.AccessibleReposEnv(ctx.User.ID)
+ if err != nil {
+ ctx.ServerError("AccessibleReposEnv", err)
+ return
+ }
+ userRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
+ if err != nil {
+ ctx.ServerError("env.RepoIDs", err)
+ return
+ }
} else {
unitType := models.UnitTypeIssues
if isPullList {
unitType = models.UnitTypePullRequests
}
- if ctxUser.IsOrganization() {
- opts.RepoSubQuery = ctxUser.OrgUnitRepositoriesSubQuery(ctx.User.ID, unitType)
- } else {
- opts.RepoSubQuery = ctxUser.UnitRepositoriesSubQuery(unitType)
+ userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
+ if err != nil {
+ ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
+ return
}
}
+ if len(userRepoIDs) == 0 {
+ userRepoIDs = []int64{-1}
+ }
+
+ opts := &models.IssuesOptions{
+ IsClosed: util.OptionalBoolOf(isShowClosed),
+ IsPull: util.OptionalBoolOf(isPullList),
+ SortType: sortType,
+ }
+
+ if repoID > 0 {
+ opts.RepoIDs = []int64{repoID}
+ }
switch filterMode {
+ case models.FilterModeAll:
+ if repoID > 0 {
+ if !com.IsSliceContainsInt64(userRepoIDs, repoID) {
+ // force an empty result
+ opts.RepoIDs = []int64{-1}
+ }
+ } else {
+ opts.RepoIDs = userRepoIDs
+ }
case models.FilterModeAssign:
opts.AssigneeID = ctxUser.ID
case models.FilterModeCreate:
@@ -242,6 +252,14 @@ func Issues(ctx *context.Context) {
opts.MentionedID = ctxUser.ID
}
+ counts, err := models.CountIssuesByRepo(opts)
+ if err != nil {
+ ctx.ServerError("CountIssuesByRepo", err)
+ return
+ }
+
+ opts.Page = page
+ opts.PageSize = setting.UI.IssuePagingNum
var labelIDs []int64
selectLabels := ctx.Query("labels")
if len(selectLabels) > 0 && selectLabels != "0" {
@@ -253,15 +271,6 @@ func Issues(ctx *context.Context) {
}
opts.LabelIDs = labelIDs
- counts, err := models.CountIssuesByRepo(opts)
- if err != nil {
- ctx.ServerError("CountIssuesByRepo", err)
- return
- }
-
- opts.Page = page
- opts.PageSize = setting.UI.IssuePagingNum
-
issues, err := models.Issues(opts)
if err != nil {
ctx.ServerError("Issues", err)
@@ -278,6 +287,41 @@ func Issues(ctx *context.Context) {
showReposMap[repoID] = repo
}
+ if repoID > 0 {
+ if _, ok := showReposMap[repoID]; !ok {
+ repo, err := models.GetRepositoryByID(repoID)
+ if models.IsErrRepoNotExist(err) {
+ ctx.NotFound("GetRepositoryByID", err)
+ return
+ } else if err != nil {
+ ctx.ServerError("GetRepositoryByID", fmt.Errorf("[%d]%v", repoID, err))
+ return
+ }
+ showReposMap[repoID] = repo
+ }
+
+ repo := showReposMap[repoID]
+
+ // Check if user has access to given repository.
+ perm, err := models.GetUserRepoPermission(repo, ctxUser)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", fmt.Errorf("[%d]%v", repoID, err))
+ return
+ }
+ if !perm.CanRead(models.UnitTypeIssues) {
+ if log.IsTrace() {
+ log.Trace("Permission Denied: User %-v cannot read %-v of repo %-v\n"+
+ "User in repo has Permissions: %-+v",
+ ctxUser,
+ models.UnitTypeIssues,
+ repo,
+ perm)
+ }
+ ctx.Status(404)
+ return
+ }
+ }
+
showRepos := models.RepositoryListOfMap(showReposMap)
sort.Sort(showRepos)
if err = showRepos.LoadAttributes(); err != nil {
@@ -295,12 +339,12 @@ func Issues(ctx *context.Context) {
}
issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
- UserID: ctxUser.ID,
- RepoID: repoID,
- RepoSubQuery: opts.RepoSubQuery,
- FilterMode: filterMode,
- IsPull: isPullList,
- IsClosed: isShowClosed,
+ UserID: ctxUser.ID,
+ RepoID: repoID,
+ UserRepoIDs: userRepoIDs,
+ FilterMode: filterMode,
+ IsPull: isPullList,
+ IsClosed: isShowClosed,
})
if err != nil {
ctx.ServerError("GetUserIssueStats", err)