Browse Source

Fix assigned issues dashboard (#920)

* Fix assigned/created issues in dashboard. (#3560)

* Fix assigned/created issues in dashboard.

* Use GetUserIssueStats for getting all Dashboard stats.

* Use gofmt to format the file properly.

* Replace &Issue{} with new(Issue).

* Check if user has access to given repository.

* Remove unnecessary filtering of issues.

* Return 404 error if invalid repository is given.

* Use correct number of issues in paginater.

* fix issues on dashboard
Lunny Xiao 3 years ago
parent
commit
7a9a5c8a69
4 changed files with 175 additions and 117 deletions
  1. 61 28
      models/issue.go
  2. 1 22
      routers/repo/issue.go
  3. 109 63
      routers/user/home.go
  4. 4 4
      templates/user/dashboard/issues.tmpl

+ 61 - 28
models/issue.go

@@ -1184,7 +1184,7 @@ func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error {
1184 1184
 // IssueStats represents issue statistic information.
1185 1185
 type IssueStats struct {
1186 1186
 	OpenCount, ClosedCount int64
1187
-	AllCount               int64
1187
+	YourRepositoriesCount  int64
1188 1188
 	AssignCount            int64
1189 1189
 	CreateCount            int64
1190 1190
 	MentionCount           int64
@@ -1210,6 +1210,7 @@ func parseCountResult(results []map[string][]byte) int64 {
1210 1210
 
1211 1211
 // IssueStatsOptions contains parameters accepted by GetIssueStats.
1212 1212
 type IssueStatsOptions struct {
1213
+	FilterMode  int
1213 1214
 	RepoID      int64
1214 1215
 	Labels      string
1215 1216
 	MilestoneID int64
@@ -1265,19 +1266,41 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
1265 1266
 	}
1266 1267
 
1267 1268
 	var err error
1268
-	stats.OpenCount, err = countSession(opts).
1269
-		And("is_closed = ?", false).
1270
-		Count(&Issue{})
1271
-	if err != nil {
1272
-		return nil, err
1273
-	}
1274
-	stats.ClosedCount, err = countSession(opts).
1275
-		And("is_closed = ?", true).
1276
-		Count(&Issue{})
1277
-	if err != nil {
1278
-		return nil, err
1269
+	switch opts.FilterMode {
1270
+	case FilterModeAll, FilterModeAssign:
1271
+		stats.OpenCount, err = countSession(opts).
1272
+			And("is_closed = ?", false).
1273
+			Count(new(Issue))
1274
+
1275
+		stats.ClosedCount, err = countSession(opts).
1276
+			And("is_closed = ?", true).
1277
+			Count(new(Issue))
1278
+	case FilterModeCreate:
1279
+		stats.OpenCount, err = countSession(opts).
1280
+			And("poster_id = ?", opts.PosterID).
1281
+			And("is_closed = ?", false).
1282
+			Count(new(Issue))
1283
+
1284
+		stats.ClosedCount, err = countSession(opts).
1285
+			And("poster_id = ?", opts.PosterID).
1286
+			And("is_closed = ?", true).
1287
+			Count(new(Issue))
1288
+	case FilterModeMention:
1289
+		stats.OpenCount, err = countSession(opts).
1290
+			Join("INNER", "issue_user", "issue.id = issue_user.issue_id").
1291
+			And("issue_user.uid = ?", opts.PosterID).
1292
+			And("issue_user.is_mentioned = ?", true).
1293
+			And("issue.is_closed = ?", false).
1294
+			Count(new(Issue))
1295
+
1296
+		stats.ClosedCount, err = countSession(opts).
1297
+			Join("INNER", "issue_user", "issue.id = issue_user.issue_id").
1298
+			And("issue_user.uid = ?", opts.PosterID).
1299
+			And("issue_user.is_mentioned = ?", true).
1300
+			And("issue.is_closed = ?", true).
1301
+			Count(new(Issue))
1279 1302
 	}
1280
-	return stats, nil
1303
+	return stats, err
1281 1304
 }
1282 1305
 
1283 1306
 // GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1298,29 +1321,39 @@ func GetUserIssueStats(repoID, uid int64, repoIDs []int64, filterMode int, isPul
1298 1321
 		return sess
1299 1322
 	}
1300 1323
 
1301
-	stats.AssignCount, _ = countSession(false, isPull, repoID, repoIDs).
1324
+	stats.AssignCount, _ = countSession(false, isPull, repoID, nil).
1302 1325
 		And("assignee_id = ?", uid).
1303
-		Count(&Issue{})
1326
+		Count(new(Issue))
1304 1327
 
1305
-	stats.CreateCount, _ = countSession(false, isPull, repoID, repoIDs).
1328
+	stats.CreateCount, _ = countSession(false, isPull, repoID, nil).
1306 1329
 		And("poster_id = ?", uid).
1307
-		Count(&Issue{})
1330
+		Count(new(Issue))
1308 1331
 
1309
-	openCountSession := countSession(false, isPull, repoID, repoIDs)
1310
-	closedCountSession := countSession(true, isPull, repoID, repoIDs)
1332
+	stats.YourRepositoriesCount, _ = countSession(false, isPull, repoID, repoIDs).
1333
+		Count(new(Issue))
1311 1334
 
1312 1335
 	switch filterMode {
1336
+	case FilterModeAll:
1337
+		stats.OpenCount, _ = countSession(false, isPull, repoID, repoIDs).
1338
+			Count(new(Issue))
1339
+		stats.ClosedCount, _ = countSession(true, isPull, repoID, repoIDs).
1340
+			Count(new(Issue))
1313 1341
 	case FilterModeAssign:
1314
-		openCountSession.And("assignee_id = ?", uid)
1315
-		closedCountSession.And("assignee_id = ?", uid)
1342
+		stats.OpenCount, _ = countSession(false, isPull, repoID, nil).
1343
+			And("assignee_id = ?", uid).
1344
+			Count(new(Issue))
1345
+		stats.ClosedCount, _ = countSession(true, isPull, repoID, nil).
1346
+			And("assignee_id = ?", uid).
1347
+			Count(new(Issue))
1316 1348
 	case FilterModeCreate:
1317
-		openCountSession.And("poster_id = ?", uid)
1318
-		closedCountSession.And("poster_id = ?", uid)
1349
+		stats.OpenCount, _ = countSession(false, isPull, repoID, nil).
1350
+			And("poster_id = ?", uid).
1351
+			Count(new(Issue))
1352
+		stats.ClosedCount, _ = countSession(true, isPull, repoID, nil).
1353
+			And("poster_id = ?", uid).
1354
+			Count(new(Issue))
1319 1355
 	}
1320 1356
 
1321
-	stats.OpenCount, _ = openCountSession.Count(&Issue{})
1322
-	stats.ClosedCount, _ = closedCountSession.Count(&Issue{})
1323
-
1324 1357
 	return stats
1325 1358
 }
1326 1359
 
@@ -1347,8 +1380,8 @@ func GetRepoIssueStats(repoID, uid int64, filterMode int, isPull bool) (numOpen
1347 1380
 		closedCountSession.And("poster_id = ?", uid)
1348 1381
 	}
1349 1382
 
1350
-	openResult, _ := openCountSession.Count(&Issue{})
1351
-	closedResult, _ := closedCountSession.Count(&Issue{})
1383
+	openResult, _ := openCountSession.Count(new(Issue))
1384
+	closedResult, _ := closedCountSession.Count(new(Issue))
1352 1385
 
1353 1386
 	return openResult, closedResult
1354 1387
 }

+ 1 - 22
routers/repo/issue.go

@@ -10,7 +10,6 @@ import (
10 10
 	"fmt"
11 11
 	"io"
12 12
 	"io/ioutil"
13
-	"net/url"
14 13
 	"strings"
15 14
 	"time"
16 15
 
@@ -108,37 +107,17 @@ func Issues(ctx *context.Context) {
108 107
 
109 108
 	viewType := ctx.Query("type")
110 109
 	sortType := ctx.Query("sort")
111
-	types := []string{"assigned", "created_by", "mentioned"}
110
+	types := []string{"all", "assigned", "created_by", "mentioned"}
112 111
 	if !com.IsSliceContainsStr(types, viewType) {
113 112
 		viewType = "all"
114 113
 	}
115 114
 
116
-	// Must sign in to see issues about you.
117
-	if viewType != "all" && !ctx.IsSigned {
118
-		ctx.SetCookie("redirect_to", "/"+url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
119
-		ctx.Redirect(setting.AppSubURL + "/user/login")
120
-		return
121
-	}
122
-
123 115
 	var (
124 116
 		assigneeID  = ctx.QueryInt64("assignee")
125 117
 		posterID    int64
126 118
 		mentionedID int64
127 119
 		forceEmpty  bool
128 120
 	)
129
-	switch viewType {
130
-	case "assigned":
131
-		if assigneeID > 0 && ctx.User.ID != assigneeID {
132
-			// two different assignees, must be empty
133
-			forceEmpty = true
134
-		} else {
135
-			assigneeID = ctx.User.ID
136
-		}
137
-	case "created_by":
138
-		posterID = ctx.User.ID
139
-	case "mentioned":
140
-		mentionedID = ctx.User.ID
141
-	}
142 121
 
143 122
 	repo := ctx.Repo.Repository
144 123
 	selectLabels := ctx.Query("labels")

+ 109 - 63
routers/user/home.go

@@ -183,34 +183,39 @@ func Issues(ctx *context.Context) {
183 183
 		viewType   string
184 184
 		sortType   = ctx.Query("sort")
185 185
 		filterMode = models.FilterModeAll
186
-		assigneeID int64
187
-		posterID   int64
188 186
 	)
187
+
189 188
 	if ctxUser.IsOrganization() {
190 189
 		viewType = "all"
191 190
 	} else {
192 191
 		viewType = ctx.Query("type")
193
-		types := []string{"assigned", "created_by"}
192
+		types := []string{"all", "assigned", "created_by"}
194 193
 		if !com.IsSliceContainsStr(types, viewType) {
195 194
 			viewType = "all"
196 195
 		}
197 196
 
198 197
 		switch viewType {
198
+		case "all":
199
+			filterMode = models.FilterModeAll
199 200
 		case "assigned":
200 201
 			filterMode = models.FilterModeAssign
201
-			assigneeID = ctxUser.ID
202 202
 		case "created_by":
203 203
 			filterMode = models.FilterModeCreate
204
-			posterID = ctxUser.ID
205 204
 		}
206 205
 	}
207 206
 
207
+	page := ctx.QueryInt("page")
208
+	if page <= 1 {
209
+		page = 1
210
+	}
211
+
208 212
 	repoID := ctx.QueryInt64("repo")
209 213
 	isShowClosed := ctx.Query("state") == "closed"
210 214
 
211 215
 	// Get repositories.
212 216
 	var err error
213 217
 	var repos []*models.Repository
218
+	userRepoIDs := make([]int64, 0, len(repos))
214 219
 	if ctxUser.IsOrganization() {
215 220
 		env, err := ctxUser.AccessibleReposEnv(ctx.User.ID)
216 221
 		if err != nil {
@@ -230,9 +235,6 @@ func Issues(ctx *context.Context) {
230 235
 		repos = ctxUser.Repos
231 236
 	}
232 237
 
233
-	allCount := 0
234
-	repoIDs := make([]int64, 0, len(repos))
235
-	showRepos := make([]*models.Repository, 0, len(repos))
236 238
 	for _, repo := range repos {
237 239
 		if (isPullList && repo.NumPulls == 0) ||
238 240
 			(!isPullList &&
@@ -240,85 +242,129 @@ func Issues(ctx *context.Context) {
240 242
 			continue
241 243
 		}
242 244
 
243
-		repoIDs = append(repoIDs, repo.ID)
245
+		userRepoIDs = append(userRepoIDs, repo.ID)
246
+	}
244 247
 
245
-		if isPullList {
246
-			allCount += repo.NumOpenPulls
247
-			repo.NumOpenIssues = repo.NumOpenPulls
248
-			repo.NumClosedIssues = repo.NumClosedPulls
249
-		} else {
250
-			allCount += repo.NumOpenIssues
248
+	var issues []*models.Issue
249
+	switch filterMode {
250
+	case models.FilterModeAll:
251
+		// Get all issues from repositories from this user.
252
+		issues, err = models.Issues(&models.IssuesOptions{
253
+			RepoIDs:  userRepoIDs,
254
+			RepoID:   repoID,
255
+			Page:     page,
256
+			IsClosed: util.OptionalBoolOf(isShowClosed),
257
+			IsPull:   util.OptionalBoolOf(isPullList),
258
+			SortType: sortType,
259
+		})
260
+
261
+	case models.FilterModeAssign:
262
+		// Get all issues assigned to this user.
263
+		issues, err = models.Issues(&models.IssuesOptions{
264
+			RepoID:     repoID,
265
+			AssigneeID: ctxUser.ID,
266
+			Page:       page,
267
+			IsClosed:   util.OptionalBoolOf(isShowClosed),
268
+			IsPull:     util.OptionalBoolOf(isPullList),
269
+			SortType:   sortType,
270
+		})
271
+
272
+	case models.FilterModeCreate:
273
+		// Get all issues created by this user.
274
+		issues, err = models.Issues(&models.IssuesOptions{
275
+			RepoID:   repoID,
276
+			PosterID: ctxUser.ID,
277
+			Page:     page,
278
+			IsClosed: util.OptionalBoolOf(isShowClosed),
279
+			IsPull:   util.OptionalBoolOf(isPullList),
280
+			SortType: sortType,
281
+		})
282
+	case models.FilterModeMention:
283
+		// Get all issues created by this user.
284
+		issues, err = models.Issues(&models.IssuesOptions{
285
+			RepoID:      repoID,
286
+			MentionedID: ctxUser.ID,
287
+			Page:        page,
288
+			IsClosed:    util.OptionalBoolOf(isShowClosed),
289
+			IsPull:      util.OptionalBoolOf(isPullList),
290
+			SortType:    sortType,
291
+		})
292
+	}
293
+
294
+	if err != nil {
295
+		ctx.Handle(500, "Issues", err)
296
+		return
297
+	}
298
+
299
+	showRepos := make([]*models.Repository, 0, len(issues))
300
+	showReposSet := make(map[int64]bool)
301
+
302
+	if repoID > 0 {
303
+		repo, err := models.GetRepositoryByID(repoID)
304
+		if err != nil {
305
+			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", repoID, err))
306
+			return
251 307
 		}
252 308
 
253
-		if filterMode != models.FilterModeAll {
254
-			// Calculate repository issue count with filter mode.
255
-			numOpen, numClosed := repo.IssueStats(ctxUser.ID, filterMode, isPullList)
256
-			repo.NumOpenIssues, repo.NumClosedIssues = int(numOpen), int(numClosed)
309
+		if err = repo.GetOwner(); err != nil {
310
+			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", repoID, err))
311
+			return
257 312
 		}
258 313
 
259
-		if repo.ID == repoID ||
260
-			(isShowClosed && repo.NumClosedIssues > 0) ||
261
-			(!isShowClosed && repo.NumOpenIssues > 0) {
262
-			showRepos = append(showRepos, repo)
314
+		// Check if user has access to given repository.
315
+		if !repo.IsOwnedBy(ctxUser.ID) && !repo.HasAccess(ctxUser) {
316
+			ctx.Handle(404, "Issues", fmt.Errorf("#%d", repoID))
317
+			return
263 318
 		}
319
+
320
+		showReposSet[repoID] = true
321
+		showRepos = append(showRepos, repo)
264 322
 	}
265
-	ctx.Data["Repos"] = showRepos
266
-	if len(repoIDs) == 0 {
267
-		repoIDs = []int64{-1}
268
-	}
269 323
 
270
-	issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, repoIDs, filterMode, isPullList)
271
-	issueStats.AllCount = int64(allCount)
324
+	for _, issue := range issues {
325
+		// Get Repository data.
326
+		issue.Repo, err = models.GetRepositoryByID(issue.RepoID)
327
+		if err != nil {
328
+			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", issue.RepoID, err))
329
+			return
330
+		}
331
+
332
+		// Get Owner data.
333
+		if err = issue.Repo.GetOwner(); err != nil {
334
+			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issue.RepoID, err))
335
+			return
336
+		}
272 337
 
273
-	page := ctx.QueryInt("page")
274
-	if page <= 1 {
275
-		page = 1
338
+		// Append repo to list of shown repos
339
+		if filterMode == models.FilterModeAll {
340
+			// Use a map to make sure we don't add the same Repository twice.
341
+			_, ok := showReposSet[issue.RepoID]
342
+			if !ok {
343
+				showReposSet[issue.RepoID] = true
344
+				// Append to list of shown Repositories.
345
+				showRepos = append(showRepos, issue.Repo)
346
+			}
347
+		}
276 348
 	}
277 349
 
350
+	issueStats := models.GetUserIssueStats(repoID, ctxUser.ID, userRepoIDs, filterMode, isPullList)
351
+
278 352
 	var total int
279 353
 	if !isShowClosed {
280 354
 		total = int(issueStats.OpenCount)
281 355
 	} else {
282 356
 		total = int(issueStats.ClosedCount)
283 357
 	}
284
-	ctx.Data["Page"] = paginater.New(total, setting.UI.IssuePagingNum, page, 5)
285
-
286
-	// Get issues.
287
-	issues, err := models.Issues(&models.IssuesOptions{
288
-		AssigneeID: assigneeID,
289
-		RepoID:     repoID,
290
-		PosterID:   posterID,
291
-		RepoIDs:    repoIDs,
292
-		Page:       page,
293
-		IsClosed:   util.OptionalBoolOf(isShowClosed),
294
-		IsPull:     util.OptionalBoolOf(isPullList),
295
-		SortType:   sortType,
296
-	})
297
-	if err != nil {
298
-		ctx.Handle(500, "Issues", err)
299
-		return
300
-	}
301 358
 
302
-	// Get posters and repository.
303
-	for i := range issues {
304
-		issues[i].Repo, err = models.GetRepositoryByID(issues[i].RepoID)
305
-		if err != nil {
306
-			ctx.Handle(500, "GetRepositoryByID", fmt.Errorf("[#%d]%v", issues[i].ID, err))
307
-			return
308
-		}
309
-
310
-		if err = issues[i].Repo.GetOwner(); err != nil {
311
-			ctx.Handle(500, "GetOwner", fmt.Errorf("[#%d]%v", issues[i].ID, err))
312
-			return
313
-		}
314
-	}
315 359
 	ctx.Data["Issues"] = issues
316
-
360
+	ctx.Data["Repos"] = showRepos
361
+	ctx.Data["Page"] = paginater.New(total, setting.UI.IssuePagingNum, page, 5)
317 362
 	ctx.Data["IssueStats"] = issueStats
318 363
 	ctx.Data["ViewType"] = viewType
319 364
 	ctx.Data["SortType"] = sortType
320 365
 	ctx.Data["RepoID"] = repoID
321 366
 	ctx.Data["IsShowClosed"] = isShowClosed
367
+
322 368
 	if isShowClosed {
323 369
 		ctx.Data["State"] = "closed"
324 370
 	} else {

+ 4 - 4
templates/user/dashboard/issues.tmpl

@@ -5,9 +5,9 @@
5 5
 		<div class="ui grid">
6 6
 			<div class="four wide column">
7 7
 				<div class="ui secondary vertical filter menu">
8
-					<a class="{{if eq .ViewType "all"}}ui basic blue button{{end}} item" href="{{.Link}}?repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
8
+					<a class="{{if eq .ViewType "your_repositories"}}ui basic blue button{{end}} item" href="{{.Link}}?type=your_repositories&repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
9 9
 						{{.i18n.Tr "home.issues.in_your_repos"}}
10
-						<strong class="ui right">{{.IssueStats.AllCount}}</strong>
10
+						<strong class="ui right">{{.IssueStats.YourRepositoriesCount}}</strong>
11 11
 					</a>
12 12
 					{{if not .ContextUser.IsOrganization}}
13 13
 						<a class="{{if eq .ViewType "assigned"}}ui basic blue button{{end}} item" href="{{.Link}}?type=assigned&repo={{.RepoID}}&sort={{$.SortType}}&state={{.State}}">
@@ -22,7 +22,7 @@
22 22
 					<div class="ui divider"></div>
23 23
 					{{range .Repos}}
24 24
 						<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}}">
25
-							<span class="text truncate">{{$.ContextUser.Name}}/{{.Name}}</span>
25
+							<span class="text truncate">{{.FullName}}</span>
26 26
 							<div class="floating ui {{if $.IsShowClosed}}red{{else}}green{{end}} label">{{if $.IsShowClosed}}{{.NumClosedIssues}}{{else}}{{.NumOpenIssues}}{{end}}</div>
27 27
 						</a>
28 28
 					{{end}}
@@ -61,7 +61,7 @@
61 61
 					{{range .Issues}}
62 62
 						{{ $timeStr:= TimeSince .Created $.Lang }}
63 63
 						<li class="item">
64
-							<div class="ui label">{{if not $.RepoID}}{{.Repo.Name}}{{end}}#{{.Index}}</div>
64
+							<div class="ui label">{{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}</div>
65 65
 							<a class="title has-emoji" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/issues/{{.Index}}">{{.Title}}</a>
66 66
 
67 67
 							{{range .Labels}}