Skip to content

Commit

Permalink
[UI] IssuePage multi repo select (#8741)
Browse files Browse the repository at this point in the history
* Make repository list @ issues/PR page persist

Will partially fix #6355

* Enable multiple selections in repo list @ issues/PR page

Part of issue #6355

* Add RepoIDs to UserIssueStatsOptions to make "type" count correct when selecting one/multiple repos.
* Replace variable "repo" with list "repos[]" and enable multiple selections of repositories from list by including/excluding RepoIDs in list.
*

* Remove redundant code

* Add 'All' button 

Improves functionality of the page, so that backtracking is not necessary to reset the page

* Remove redundant variable

Completely replace 'RepoID' with 'RepoIDs' and remove redundant code

* Add RepoIDs to label link

* Revert part of code to previous version to troubleshoot build failure

* Implement old and new pieces of code whilst adhering to multi select

* Attempt to join the two versions

Last commit passed tests but doesn't work in practice, this works in practice and hopefully passes the tests.

* Update tests to desired state

* Fix pagination implementation and tests

* Pass repoIDs as `repos=[1,2,3...]` instead of several `repos[]=..`
* Update tests file to reflect new functionality
* Update template with new `repos` format
* Implement new solution to show constant "total issues" count for "All" button

* Correct behavior when passing zero to array

* Comment out test url returning 404

This keeps returning 404 in the test despite working in practice, for the sake of running more tests I am commenting it out

* Comment out another test url returning 404

Last attempt, if more tests crash I will uncomment the urls and request assistance.

* Reenable tests and test fix

* Re-enable tests
* Make selecting "In your repositories" reset selection as passing IDs of repos belonging to other profiles causes breakage
* Remove unnecessary (with multi-selection enable) code

* Drop repo from repo map and total count if permission denied

* Remove extra parenthesis

* make template work again

* find bug!

* forgot the '#' at bugfixing

* delete unused RepoID

* compile regex only one time

* make fmt

* local variable = capital letter lower

* check if repos query pattern is correct

* pagination remove last , - make regex work again

* use Replace instead of ReplaceAll; del delete

* fix test

* how did this test binary got in?!?
dont forgot the "-p" at git add * !

* dont replace -> cut fisrt & last string

Co-Authored-By: zeripath <art27@cantab.net>

* jet another regex

dont mind as long as it has the same result and is performatn ...

Co-Authored-By: zeripath <art27@cantab.net>

* dont use nonexisting repo for test

* exclude /issues?type=created_by from test

* add table to querys
use same syntax in each query (table.colum)

* add new issue for test

* dont make a workaround or something else
this need a refactor itself and is out of scope for this PR

* fix misspell

* CI.redo()

* englisch txt update

Co-Authored-By: zeripath <art27@cantab.net>

* add sugestions

* Tweak & Fix

* CI.restart()
  • Loading branch information
6543 authored and techknowlogick committed Dec 2, 2019
1 parent 668eaf9 commit 66028d5
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 95 deletions.
25 changes: 13 additions & 12 deletions integrations/links_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,20 @@ func testLinksAsUser(userName string, t *testing.T) {
"/api/swagger",
"/api/v1/swagger",
"/issues",
"/issues?type=your_repositories&repo=0&sort=&state=open",
"/issues?type=assigned&repo=0&sort=&state=open",
"/issues?type=created_by&repo=0&sort=&state=open",
"/issues?type=your_repositories&repo=0&sort=&state=closed",
"/issues?type=assigned&repo=0&sort=&state=closed",
"/issues?type=created_by&repo=0&sort=&state=closed",
"/issues?type=your_repositories&repos=[0]&sort=&state=open",
"/issues?type=assigned&repos=[0]&sort=&state=open",
"/issues?type=your_repositories&repos=[0]&sort=&state=closed",
"/issues?type=assigned&repos=[]&sort=&state=closed",
"/issues?type=assigned&sort=&state=open",
"/issues?type=created_by&repos=[1,2]&sort=&state=closed",
"/issues?type=created_by&repos=[1,2]&sort=&state=open",
"/pulls",
"/pulls?type=your_repositories&repo=0&sort=&state=open",
"/pulls?type=assigned&repo=0&sort=&state=open",
"/pulls?type=created_by&repo=0&sort=&state=open",
"/pulls?type=your_repositories&repo=0&sort=&state=closed",
"/pulls?type=assigned&repo=0&sort=&state=closed",
"/pulls?type=created_by&repo=0&sort=&state=closed",
"/pulls?type=your_repositories&repos=[2]&sort=&state=open",
"/pulls?type=assigned&repos=[]&sort=&state=open",
"/pulls?type=created_by&repos=[0]&sort=&state=open",
"/pulls?type=your_repositories&repos=[0]&sort=&state=closed",
"/pulls?type=assigned&repos=[0]&sort=&state=closed",
"/pulls?type=created_by&repos=[0]&sort=&state=closed",
"/notifications",
"/repo/create",
"/repo/migrate",
Expand Down
2 changes: 1 addition & 1 deletion models/fixtures/issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,4 @@
is_closed: false
is_pull: true
created_unix: 946684820
updated_unix: 978307180
updated_unix: 978307180
4 changes: 4 additions & 0 deletions models/fixtures/issue_assignees.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
id: 2
assignee_id: 1
issue_id: 6
-
id: 3
assignee_id: 2
issue_id: 6
18 changes: 9 additions & 9 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
type UserIssueStatsOptions struct {
UserID int64
RepoID int64
RepoIDs []int64
UserRepoIDs []int64
FilterMode int
IsPull bool
Expand All @@ -1416,19 +1416,19 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {

cond := builder.NewCond()
cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull})
if opts.RepoID > 0 {
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
if len(opts.RepoIDs) > 0 {
cond = cond.And(builder.In("issue.repo_id", opts.RepoIDs))
}

switch opts.FilterMode {
case FilterModeAll:
stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
stats.OpenCount, err = x.Where(cond).And("issue.is_closed = ?", false).
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).
stats.ClosedCount, err = x.Where(cond).And("issue.is_closed = ?", true).
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
Count(new(Issue))
if err != nil {
Expand All @@ -1450,14 +1450,14 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
return nil, err
}
case FilterModeCreate:
stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
And("poster_id = ?", opts.UserID).
stats.OpenCount, err = x.Where(cond).And("issue.is_closed = ?", false).
And("issue.poster_id = ?", opts.UserID).
Count(new(Issue))
if err != nil {
return nil, err
}
stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
And("poster_id = ?", opts.UserID).
stats.ClosedCount, err = x.Where(cond).And("issue.is_closed = ?", true).
And("issue.poster_id = ?", opts.UserID).
Count(new(Issue))
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion models/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestGetUserIssueStats(t *testing.T) {
{
UserIssueStatsOptions{
UserID: 1,
RepoID: 1,
RepoIDs: []int64{1},
FilterMode: FilterModeAll,
},
IssueStats{
Expand Down
115 changes: 63 additions & 52 deletions routers/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ package user

import (
"bytes"
"encoding/json"
"fmt"
"regexp"
"sort"
"strconv"
"strings"

"code.gitea.io/gitea/models"
Expand All @@ -20,7 +23,6 @@ import (

"github.com/keybase/go-crypto/openpgp"
"github.com/keybase/go-crypto/openpgp/armor"
"github.com/unknwon/com"
)

const (
Expand Down Expand Up @@ -149,6 +151,9 @@ func Dashboard(ctx *context.Context) {
ctx.HTML(200, tplDashboard)
}

// Regexp for repos query
var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`)

// Issues render the user issues page
func Issues(ctx *context.Context) {
isPullList := ctx.Params(":type") == "pulls"
Expand Down Expand Up @@ -194,7 +199,25 @@ func Issues(ctx *context.Context) {
page = 1
}

repoID := ctx.QueryInt64("repo")
reposQuery := ctx.Query("repos")
var repoIDs []int64
if issueReposQueryPattern.MatchString(reposQuery) {
// remove "[" and "]" from string
reposQuery = reposQuery[1 : len(reposQuery)-1]
//for each ID (delimiter ",") add to int to repoIDs
for _, rID := range strings.Split(reposQuery, ",") {
// Ensure nonempty string entries
if rID != "" && rID != "0" {
rIDint64, err := strconv.ParseInt(rID, 10, 64)
if err == nil {
repoIDs = append(repoIDs, rIDint64)
}
}
}
} else {
log.Error("issueReposQueryPattern not match with query")
}

isShowClosed := ctx.Query("state") == "closed"

// Get repositories.
Expand Down Expand Up @@ -232,20 +255,9 @@ func Issues(ctx *context.Context) {
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
}
opts.RepoIDs = userRepoIDs
case models.FilterModeAssign:
opts.AssigneeID = ctxUser.ID
case models.FilterModeCreate:
Expand Down Expand Up @@ -273,6 +285,8 @@ func Issues(ctx *context.Context) {
}
opts.LabelIDs = labelIDs

opts.RepoIDs = repoIDs

issues, err := models.Issues(opts)
if err != nil {
ctx.ServerError("Issues", err)
Expand All @@ -281,46 +295,23 @@ func Issues(ctx *context.Context) {

showReposMap := make(map[int64]*models.Repository, len(counts))
for repoID := range counts {
repo, err := models.GetRepositoryByID(repoID)
if err != nil {
ctx.ServerError("GetRepositoryByID", err)
showReposMap[repoID], 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
}

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)
perm, err := models.GetUserRepoPermission(showReposMap[repoID], 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
log.Error("User created Issues in Repository which they no longer have access to: [%d]", repoID)
}
}

Expand All @@ -342,7 +333,6 @@ func Issues(ctx *context.Context) {

issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID,
RepoID: repoID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
Expand All @@ -353,11 +343,26 @@ func Issues(ctx *context.Context) {
return
}

var total int
allIssueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
})
if err != nil {
ctx.ServerError("GetUserIssueStats All", err)
return
}

var shownIssues int
var totalIssues int
if !isShowClosed {
total = int(issueStats.OpenCount)
shownIssues = int(issueStats.OpenCount)
totalIssues = int(allIssueStats.OpenCount)
} else {
total = int(issueStats.ClosedCount)
shownIssues = int(issueStats.ClosedCount)
totalIssues = int(allIssueStats.ClosedCount)
}

ctx.Data["Issues"] = issues
Expand All @@ -367,18 +372,24 @@ func Issues(ctx *context.Context) {
ctx.Data["IssueStats"] = issueStats
ctx.Data["ViewType"] = viewType
ctx.Data["SortType"] = sortType
ctx.Data["RepoID"] = repoID
ctx.Data["RepoIDs"] = repoIDs
ctx.Data["IsShowClosed"] = isShowClosed
ctx.Data["TotalIssueCount"] = totalIssues

if isShowClosed {
ctx.Data["State"] = "closed"
} else {
ctx.Data["State"] = "open"
}

pager := context.NewPagination(total, setting.UI.IssuePagingNum, page, 5)
// Convert []int64 to string
reposParam, _ := json.Marshal(repoIDs)

ctx.Data["ReposParam"] = string(reposParam)

pager := context.NewPagination(shownIssues, setting.UI.IssuePagingNum, page, 5)
pager.AddParam(ctx, "type", "ViewType")
pager.AddParam(ctx, "repo", "RepoID")
pager.AddParam(ctx, "repos", "ReposParam")
pager.AddParam(ctx, "sort", "SortType")
pager.AddParam(ctx, "state", "State")
pager.AddParam(ctx, "labels", "SelectLabels")
Expand Down
Loading

0 comments on commit 66028d5

Please sign in to comment.