Fix all the bugs in issues and pulls on dashboard (#943)

* fix all the bugs in issues and pulls on dashboard

* small fix and refactor

* add method getRepoIDs for IssueList

models/issue.go

@@ -1399,3 +1399,62 @@ func updateIssue(e Engine, issue *Issue) error {
 func UpdateIssue(issue *Issue) error {
 	return updateIssue(x, issue)
 }
+
+// IssueList defines a list of issues
+type IssueList []*Issue
+
+func (issues IssueList) getRepoIDs() []int64 {
+	repoIDs := make([]int64, 0, len(issues))
+	for _, issue := range issues {
+		var has bool
+		for _, repoID := range repoIDs {
+			if repoID == issue.RepoID {
+				has = true
+				break
+			}
+		}
+		if !has {
+			repoIDs = append(repoIDs, issue.RepoID)
+		}
+	}
+	return repoIDs
+}
+
+func (issues IssueList) loadRepositories(e Engine) ([]*Repository, error) {
+	if len(issues) == 0 {
+		return nil, nil
+	}
+
+	repoIDs := issues.getRepoIDs()
+	rows, err := e.
+		Where("id > 0").
+		In("id", repoIDs).
+		Rows(new(Repository))
+	if err != nil {
+		return nil, fmt.Errorf("find repository: %v", err)
+	}
+	defer rows.Close()
+
+	repositories := make([]*Repository, 0, len(repoIDs))
+	repoMaps := make(map[int64]*Repository, len(repoIDs))
+	for rows.Next() {
+		var repo Repository
+		err = rows.Scan(&repo)
+		if err != nil {
+			return nil, fmt.Errorf("find repository: %v", err)
+		}
+
+		repositories = append(repositories, &repo)
+		repoMaps[repo.ID] = &repo
+	}
+
+	for _, issue := range issues {
+		issue.Repo = repoMaps[issue.RepoID]
+	}
+	return repositories, nil
+}
+
+// LoadRepositories loads issues' all repositories
+func (issues IssueList) LoadRepositories() ([]*Repository, error) {
+	return issues.loadRepositories(x)
+}

models/user.go

@@ -500,6 +500,34 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
 	return err
 }
 
+// GetRepositoryIDs returns repositories IDs where user owned
+func (u *User) GetRepositoryIDs() ([]int64, error) {
+	var ids []int64
+	return ids, x.Table("repository").Cols("id").Where("owner_id = ?", u.ID).Find(&ids)
+}
+
+// GetOrgRepositoryIDs returns repositories IDs where user's team owned
+func (u *User) GetOrgRepositoryIDs() ([]int64, error) {
+	var ids []int64
+	return ids, x.Table("repository").
+		Cols("repository.id").
+		Join("INNER", "team_user", "repository.owner_id = team_user.org_id AND team_user.uid = ?", u.ID).
+		GroupBy("repository.id").Find(&ids)
+}
+
+// GetAccessRepoIDs returns all repsitories IDs where user's or user is a team member orgnizations
+func (u *User) GetAccessRepoIDs() ([]int64, error) {
+	ids, err := u.GetRepositoryIDs()
+	if err != nil {
+		return nil, err
+	}
+	ids2, err := u.GetOrgRepositoryIDs()
+	if err != nil {
+		return nil, err
+	}
+	return append(ids, ids2...), nil
+}
+
 // GetMirrorRepositories returns mirror repositories that user owns, including private repositories.
 func (u *User) GetMirrorRepositories() ([]*Repository, error) {
 	return GetUserMirrorRepositories(u.ID)

routers/user/home.go

@@ -214,48 +214,28 @@ func Issues(ctx *context.Context) {
 
 	// Get repositories.
 	var err error
-	var repos []*models.Repository
+	var userRepoIDs []int64
-	userRepoIDs := make([]int64, 0, len(repos))
 	if ctxUser.IsOrganization() {
 		env, err := ctxUser.AccessibleReposEnv(ctx.User.ID)
 		if err != nil {
 			ctx.Handle(500, "AccessibleReposEnv", err)
 			return
 		}
-		repos, err = env.Repos(1, ctxUser.NumRepos)
+		userRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
 		if err != nil {
-			ctx.Handle(500, "GetRepositories", err)
+			ctx.Handle(500, "env.RepoIDs", err)
 			return
 		}
-
-		for _, repo := range repos {
-			if (isPullList && repo.NumPulls == 0) ||
-				(!isPullList &&
-					(!repo.EnableUnit(models.UnitTypeIssues) || repo.NumIssues == 0)) {
-				continue
-			}
-
-			userRepoIDs = append(userRepoIDs, repo.ID)
-		}
-
-		if len(userRepoIDs) <= 0 {
-			userRepoIDs = []int64{-1}
-		}
-
 	} else {
-		if err := ctxUser.GetRepositories(1, ctx.User.NumRepos); err != nil {
+		userRepoIDs, err = ctxUser.GetAccessRepoIDs()
-			ctx.Handle(500, "GetRepositories", err)
+		if err != nil {
+			ctx.Handle(500, "ctxUser.GetAccessRepoIDs", err)
 			return
 		}
-		repos = ctxUser.Repos
+	}
 
-		for _, repo := range repos {
+	if len(userRepoIDs) <= 0 {
-			if (isPullList && repo.NumPulls == 0) ||
+		userRepoIDs = []int64{-1}
-				(!isPullList &&
-					(!repo.EnableUnit(models.UnitTypeIssues) || repo.NumIssues == 0)) {
-				continue
-			}
-		}
 	}
 
 	var issues []*models.Issue
@@ -309,55 +289,41 @@ func Issues(ctx *context.Context) {
 		return
 	}
 
-	showRepos := make([]*models.Repository, 0, len(issues))
+	showRepos, err := models.IssueList(issues).LoadRepositories()
-	showReposSet := make(map[int64]bool)
+	if err != nil {
+		ctx.Handle(500, "LoadRepositories", fmt.Errorf("%v", err))
+		return
+	}
 
 	if repoID > 0 {
-		repo, err := models.GetRepositoryByID(repoID)
+		var theRepo *models.Repository
-		if err != nil {
+		for _, repo := range showRepos {
-			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err))
+			if repo.ID == repoID {
-			return
+				theRepo = repo
+				break
+			}
 		}
 
-		if err = repo.GetOwner(); err != nil {
+		if theRepo == nil {
-			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", repoID, err))
+			theRepo, err = models.GetRepositoryByID(repoID)
-			return
+			if err != nil {
+				ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err))
+				return
+			}
+			showRepos = append(showRepos, theRepo)
 		}
 
 		// Check if user has access to given repository.
-		if !repo.IsOwnedBy(ctxUser.ID) && !repo.HasAccess(ctxUser) {
+		if !theRepo.IsOwnedBy(ctxUser.ID) && !theRepo.HasAccess(ctxUser) {
 			ctx.Handle(404, "Issues", fmt.Errorf("#%d", repoID))
 			return
 		}
-
-		showReposSet[repoID] = true
-		showRepos = append(showRepos, repo)
 	}
 
-	for _, issue := range issues {
+	err = models.RepositoryList(showRepos).LoadAttributes()
-		// Get Repository data.
+	if err != nil {
-		issue.Repo, err = models.GetRepositoryByID(issue.RepoID)
+		ctx.Handle(500, "LoadAttributes", fmt.Errorf("%v", err))
-		if err != nil {
+		return
-			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", issue.RepoID, err))
-			return
-		}
-
-		// Get Owner data.
-		if err = issue.Repo.GetOwner(); err != nil {
-			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issue.RepoID, err))
-			return
-		}
-
-		// Append repo to list of shown repos
-		if filterMode == models.FilterModeAll {
-			// Use a map to make sure we don't add the same Repository twice.
-			_, ok := showReposSet[issue.RepoID]
-			if !ok {
-				showReposSet[issue.RepoID] = true
-				// Append to list of shown Repositories.
-				showRepos = append(showRepos, issue.Repo)
-			}
-		}
 	}
 
 	issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, userRepoIDs, filterMode, isPullList)

templates/user/dashboard/issues.tmpl

@@ -23,7 +23,7 @@
 					{{range .Repos}}
 						<a class="{{if eq $.RepoID .ID}}ui basic blue button{{end}} repo name item" href="{{$.Link}}?type={{$.ViewType}}{{if not (eq $.RepoID .ID)}}&repo={{.ID}}{{end}}&sort={{$.SortType}}&state={{$.State}}">
 							<span class="text truncate">{{.FullName}}</span>
-							<div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{if $.IsShowClosed}}{{.NumClosedIssues}}{{else}}{{.NumOpenIssues}}{{end}}</div>
+							<div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{if $.IsShowClosed}}{{if $.PageIsPulls}}{{.NumClosedPulls}}{{else}}{{.NumClosedIssues}}{{end}}{{else}}{{if $.PageIsPulls}}{{.NumOpenPulls}}{{else}}{{.NumOpenIssues}}{{end}}{{end}}</div>
 						</a>
 					{{end}}
 				</div>