From 66028d58f9ed3f69a82b84360784f8e6ccbcd582 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 2 Dec 2019 04:50:36 +0100 Subject: [PATCH] [UI] IssuePage multi repo select (#8741) * 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 * jet another regex dont mind as long as it has the same result and is performatn ... Co-Authored-By: zeripath * 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 * add sugestions * Tweak & Fix * CI.restart() --- integrations/links_test.go | 25 +++--- models/fixtures/issue.yml | 2 +- models/fixtures/issue_assignees.yml | 4 + models/issue.go | 18 ++--- models/issue_test.go | 2 +- routers/user/home.go | 115 +++++++++++++++------------ templates/user/dashboard/issues.tmpl | 61 +++++++++----- 7 files changed, 132 insertions(+), 95 deletions(-) diff --git a/integrations/links_test.go b/integrations/links_test.go index 97b390ed12..fc0f164552 100644 --- a/integrations/links_test.go +++ b/integrations/links_test.go @@ -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", diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index 585242a14d..541e5faf61 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -96,4 +96,4 @@ is_closed: false is_pull: true created_unix: 946684820 - updated_unix: 978307180 \ No newline at end of file + updated_unix: 978307180 diff --git a/models/fixtures/issue_assignees.yml b/models/fixtures/issue_assignees.yml index 1be07df7d4..2e89b2b0b3 100644 --- a/models/fixtures/issue_assignees.yml +++ b/models/fixtures/issue_assignees.yml @@ -6,3 +6,7 @@ id: 2 assignee_id: 1 issue_id: 6 +- + id: 3 + assignee_id: 2 + issue_id: 6 diff --git a/models/issue.go b/models/issue.go index 310971c45f..a3597f1585 100644 --- a/models/issue.go +++ b/models/issue.go @@ -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 @@ -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 { @@ -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 diff --git a/models/issue_test.go b/models/issue_test.go index d16c1de25d..d369b0acf5 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -180,7 +180,7 @@ func TestGetUserIssueStats(t *testing.T) { { UserIssueStatsOptions{ UserID: 1, - RepoID: 1, + RepoIDs: []int64{1}, FilterMode: FilterModeAll, }, IssueStats{ diff --git a/routers/user/home.go b/routers/user/home.go index 40b3bc3fc1..216f91e4db 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -7,8 +7,11 @@ package user import ( "bytes" + "encoding/json" "fmt" + "regexp" "sort" + "strconv" "strings" "code.gitea.io/gitea/models" @@ -20,7 +23,6 @@ import ( "github.com/keybase/go-crypto/openpgp" "github.com/keybase/go-crypto/openpgp/armor" - "github.com/unknwon/com" ) const ( @@ -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" @@ -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. @@ -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: @@ -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) @@ -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) } } @@ -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, @@ -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 @@ -367,8 +372,9 @@ 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" @@ -376,9 +382,14 @@ func Issues(ctx *context.Context) { 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") diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 5418c6de47..e4d09c066c 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -5,40 +5,59 @@
- + {{.i18n.Tr "home.issues.in_your_repos"}} {{.IssueStats.YourRepositoriesCount}} {{if not .ContextUser.IsOrganization}} - + {{.i18n.Tr "repo.issues.filter_type.assigned_to_you"}} {{.IssueStats.AssignCount}} - + {{.i18n.Tr "repo.issues.filter_type.created_by_you"}} {{.IssueStats.CreateCount}} - + {{.i18n.Tr "repo.issues.filter_type.mentioning_you"}} {{.IssueStats.MentionCount}} {{end}}
+ + All +
{{.TotalIssueCount}}
+ {{range .Repos}} - - {{.FullName}} -
{{index $.Counts .ID}}
- + {{with $Repo := .}} + + {{$Repo.FullName}} +
{{index $.Counts $Repo.ID}}
+ + {{end}} {{end}}
- + {{.i18n.Tr "repo.issues.open_tab" .IssueStats.OpenCount}} - + {{.i18n.Tr "repo.issues.close_tab" .IssueStats.ClosedCount}} @@ -51,14 +70,14 @@
- {{.i18n.Tr "repo.issues.filter_sort.latest"}} - {{.i18n.Tr "repo.issues.filter_sort.oldest"}} - {{.i18n.Tr "repo.issues.filter_sort.recentupdate"}} - {{.i18n.Tr "repo.issues.filter_sort.leastupdate"}} - {{.i18n.Tr "repo.issues.filter_sort.mostcomment"}} - {{.i18n.Tr "repo.issues.filter_sort.leastcomment"}} - {{.i18n.Tr "repo.issues.filter_sort.nearduedate"}} - {{.i18n.Tr "repo.issues.filter_sort.farduedate"}} + {{.i18n.Tr "repo.issues.filter_sort.latest"}} + {{.i18n.Tr "repo.issues.filter_sort.oldest"}} + {{.i18n.Tr "repo.issues.filter_sort.recentupdate"}} + {{.i18n.Tr "repo.issues.filter_sort.leastupdate"}} + {{.i18n.Tr "repo.issues.filter_sort.mostcomment"}} + {{.i18n.Tr "repo.issues.filter_sort.leastcomment"}} + {{.i18n.Tr "repo.issues.filter_sort.nearduedate"}} + {{.i18n.Tr "repo.issues.filter_sort.farduedate"}}
@@ -68,7 +87,8 @@ {{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
  • -
    {{if not $.RepoID}}{{.Repo.FullName}}{{end}}#{{.Index}}
    + {{if .Repo}} +
    {{.Repo.FullName}}#{{.Index}}
    {{.Title}} {{if .IsPull }} @@ -84,7 +104,7 @@ especially on mobile views. */}} {{range .}} - {{.Name}} + {{.Name}} {{end}} {{end}} @@ -133,6 +153,7 @@ {{end}}

    • + {{end}} {{end}} {{template "base/paginate" .}}