Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor issue list #32755

Merged
merged 6 commits into from
Dec 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func NewFuncMap() template.FuncMap {
"HTMLFormat": htmlutil.HTMLFormat,
"HTMLEscape": htmlEscape,
"QueryEscape": queryEscape,
"QueryBuild": queryBuild,
"JSEscape": jsEscapeSafe,
"SanitizeHTML": SanitizeHTML,
"URLJoin": util.URLJoin,
Expand Down Expand Up @@ -293,6 +294,71 @@ func timeEstimateString(timeSec any) string {
return util.TimeEstimateString(v)
}

type QueryString string

func queryBuild(a ...any) QueryString {
var s string
if len(a)%2 == 1 {
if v, ok := a[0].(string); ok {
if v == "" || (v[0] != '?' && v[0] != '&') {
panic("queryBuild: invalid argument")
}
s = v
} else if v, ok := a[0].(QueryString); ok {
s = string(v)
} else {
panic("queryBuild: invalid argument")
}
}
for i := len(a) % 2; i < len(a); i += 2 {
k, ok := a[i].(string)
if !ok {
panic("queryBuild: invalid argument")
}
var v string
if va, ok := a[i+1].(string); ok {
v = va
} else if a[i+1] != nil {
v = fmt.Sprint(a[i+1])
}
// pos1 to pos2 is the "k=v&" part, "&" is optional
pos1 := strings.Index(s, "&"+k+"=")
if pos1 != -1 {
pos1++
} else {
pos1 = strings.Index(s, "?"+k+"=")
if pos1 != -1 {
pos1++
} else if strings.HasPrefix(s, k+"=") {
pos1 = 0
}
}
pos2 := len(s)
if pos1 == -1 {
pos1 = len(s)
} else {
pos2 = pos1 + 1
for pos2 < len(s) && s[pos2-1] != '&' {
pos2++
}
}
if v != "" {
sep := ""
hasPrefixSep := pos1 == 0 || (pos1 <= len(s) && (s[pos1-1] == '?' || s[pos1-1] == '&'))
if !hasPrefixSep {
sep = "&"
}
s = s[:pos1] + sep + k + "=" + url.QueryEscape(v) + "&" + s[pos2:]
} else {
s = s[:pos1] + s[pos2:]
}
}
if s != "" && s != "&" && s[len(s)-1] == '&' {
s = s[:len(s)-1]
}
return QueryString(s)
}

func panicIfDevOrTesting() {
if !setting.IsProd || setting.IsInTesting {
panic("legacy template functions are for backward compatibility only, do not use them in new code")
Expand Down
31 changes: 14 additions & 17 deletions routers/web/repo/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,19 +504,16 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
if !util.SliceContainsString(types, viewType, true) {
viewType = "all"
}

var (
assigneeID = ctx.FormInt64("assignee")
posterID = ctx.FormInt64("poster")
mentionedID int64
reviewRequestedID int64
reviewedID int64
)
// TODO: "assignee" should also use GetFilterUserIDByName in the future to support usernames directly
assigneeID := ctx.FormInt64("assignee")
posterUsername := ctx.FormString("poster")
posterUserID := shared_user.GetFilterUserIDByName(ctx, posterUsername)
var mentionedID, reviewRequestedID, reviewedID int64

if ctx.IsSigned {
switch viewType {
case "created_by":
posterID = ctx.Doer.ID
posterUserID = ctx.Doer.ID
case "mentioned":
mentionedID = ctx.Doer.ID
case "assigned":
Expand Down Expand Up @@ -564,7 +561,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
ProjectID: projectID,
AssigneeID: assigneeID,
MentionedID: mentionedID,
PosterID: posterID,
PosterID: posterUserID,
ReviewRequestedID: reviewRequestedID,
ReviewedID: reviewedID,
IsPull: isPullOption,
Expand Down Expand Up @@ -646,7 +643,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
},
RepoIDs: []int64{repo.ID},
AssigneeID: assigneeID,
PosterID: posterID,
PosterID: posterUserID,
MentionedID: mentionedID,
ReviewRequestedID: reviewRequestedID,
ReviewedID: reviewedID,
Expand Down Expand Up @@ -800,24 +797,24 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
ctx.Data["IssueStats"] = issueStats
ctx.Data["OpenCount"] = issueStats.OpenCount
ctx.Data["ClosedCount"] = issueStats.ClosedCount
linkStr := "%s?q=%s&type=%s&sort=%s&state=%s&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%d&archived=%t"
linkStr := "%s?q=%s&type=%s&sort=%s&state=%s&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%v&archived=%t"
ctx.Data["AllStatesLink"] = fmt.Sprintf(linkStr, ctx.Link,
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "all", url.QueryEscape(selectLabels),
milestoneID, projectID, assigneeID, posterID, archived)
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Link,
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "open", url.QueryEscape(selectLabels),
milestoneID, projectID, assigneeID, posterID, archived)
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
ctx.Data["ClosedLink"] = fmt.Sprintf(linkStr, ctx.Link,
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "closed", url.QueryEscape(selectLabels),
milestoneID, projectID, assigneeID, posterID, archived)
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
ctx.Data["SelLabelIDs"] = labelIDs
ctx.Data["SelectLabels"] = selectLabels
ctx.Data["ViewType"] = viewType
ctx.Data["SortType"] = sortType
ctx.Data["MilestoneID"] = milestoneID
ctx.Data["ProjectID"] = projectID
ctx.Data["AssigneeID"] = assigneeID
ctx.Data["PosterID"] = posterID
ctx.Data["PosterUsername"] = posterUsername
ctx.Data["Keyword"] = keyword
ctx.Data["IsShowClosed"] = isShowClosed
switch {
Expand All @@ -838,7 +835,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
pager.AddParamString("milestone", fmt.Sprint(milestoneID))
pager.AddParamString("project", fmt.Sprint(projectID))
pager.AddParamString("assignee", fmt.Sprint(assigneeID))
pager.AddParamString("poster", fmt.Sprint(posterID))
pager.AddParamString("poster", posterUsername)
pager.AddParamString("archived", fmt.Sprint(archived))

ctx.Data["Page"] = pager
Expand Down
21 changes: 21 additions & 0 deletions routers/web/shared/user/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
package user

import (
"context"
"slices"
"strconv"

"code.gitea.io/gitea/models/user"
)
Expand All @@ -24,3 +26,22 @@ func MakeSelfOnTop(doer *user.User, users []*user.User) []*user.User {
}
return users
}

// GetFilterUserIDByName tries to get the user ID from the given username.
// Before, the "issue filter" passes user ID to query the list, but in many cases, it's impossible to pre-fetch the full user list.
// So it's better to make it work like GitHub: users could input username directly.
// Since it only converts the username to ID directly and is only used internally (to search issues), so no permission check is needed.
// Old usage: poster=123, new usage: poster=the-username (at the moment, non-existing username is treated as poster=0, not ideal but acceptable)
func GetFilterUserIDByName(ctx context.Context, name string) int64 {
if name == "" {
return 0
}
u, err := user.GetUserByName(ctx, name)
if err != nil {
if id, err := strconv.ParseInt(name, 10, 64); err == nil {
return id
}
return 0
}
return u.ID
}
70 changes: 38 additions & 32 deletions routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import (
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/web/feed"
"code.gitea.io/gitea/routers/web/shared/user"
"code.gitea.io/gitea/services/context"
feed_service "code.gitea.io/gitea/services/feed"
issue_service "code.gitea.io/gitea/services/issue"
Expand Down Expand Up @@ -375,16 +377,8 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
return
}

var (
viewType string
sortType = ctx.FormString("sort")
filterMode int
)

// Default to recently updated, unlike repository issues list
if sortType == "" {
sortType = "recentupdate"
}
sortType := util.IfZero(ctx.FormString("sort"), "recentupdate")

// --------------------------------------------------------------------------------
// Distinguish User from Organization.
Expand All @@ -399,7 +393,8 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {

// TODO: distinguish during routing

viewType = ctx.FormString("type")
viewType := ctx.FormString("type")
var filterMode int
switch viewType {
case "assigned":
filterMode = issues_model.FilterModeAssign
Expand Down Expand Up @@ -443,6 +438,14 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
Team: team,
User: ctx.Doer,
}
// Get filter by author id & assignee id
// FIXME: this feature doesn't work at the moment, because frontend can't use a "user-remote-search" dropdown directly
// the existing "/posters" handlers doesn't work for this case, it is unable to list the related users correctly.
// In the future, we need something like github: "author:user1" to accept usernames directly.
posterUsername := ctx.FormString("poster")
opts.PosterID = user.GetFilterUserIDByName(ctx, posterUsername)
// TODO: "assignee" should also use GetFilterUserIDByName in the future to support usernames directly
opts.AssigneeID, _ = strconv.ParseInt(ctx.FormString("assignee"), 10, 64)

isFuzzy := ctx.FormBool("fuzzy")

Expand Down Expand Up @@ -573,8 +576,22 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
// -------------------------------
// Fill stats to post to ctx.Data.
// -------------------------------
issueStats, err := getUserIssueStats(ctx, ctxUser, filterMode, issue_indexer.ToSearchOptions(keyword, opts).Copy(
func(o *issue_indexer.SearchOptions) { o.IsFuzzyKeyword = isFuzzy },
issueStats, err := getUserIssueStats(ctx, filterMode, issue_indexer.ToSearchOptions(keyword, opts).Copy(
func(o *issue_indexer.SearchOptions) {
o.IsFuzzyKeyword = isFuzzy
// If the doer is the same as the context user, which means the doer is viewing his own dashboard,
// it's not enough to show the repos that the doer owns or has been explicitly granted access to,
// because the doer may create issues or be mentioned in any public repo.
// So we need search issues in all public repos.
o.AllPublic = ctx.Doer.ID == ctxUser.ID
// TODO: to make it work with poster/assignee filter, then these IDs should be kept
o.AssigneeID = nil
o.PosterID = nil

o.MentionID = nil
o.ReviewRequestedID = nil
o.ReviewedID = nil
},
))
if err != nil {
ctx.ServerError("getUserIssueStats", err)
Expand Down Expand Up @@ -630,6 +647,8 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
ctx.Data["IsShowClosed"] = isShowClosed
ctx.Data["SelectLabels"] = selectedLabels
ctx.Data["IsFuzzy"] = isFuzzy
ctx.Data["SearchFilterPosterID"] = util.Iif[any](opts.PosterID != 0, opts.PosterID, nil)
ctx.Data["SearchFilterAssigneeID"] = util.Iif[any](opts.AssigneeID != 0, opts.AssigneeID, nil)

if isShowClosed {
ctx.Data["State"] = "closed"
Expand All @@ -643,7 +662,11 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
pager.AddParamString("sort", sortType)
pager.AddParamString("state", fmt.Sprint(ctx.Data["State"]))
pager.AddParamString("labels", selectedLabels)
pager.AddParamString("fuzzy", fmt.Sprintf("%v", isFuzzy))
pager.AddParamString("fuzzy", fmt.Sprint(isFuzzy))
pager.AddParamString("poster", posterUsername)
if opts.AssigneeID != 0 {
pager.AddParamString("assignee", fmt.Sprint(opts.AssigneeID))
}
ctx.Data["Page"] = pager

ctx.HTML(http.StatusOK, tplIssues)
Expand Down Expand Up @@ -768,27 +791,10 @@ func UsernameSubRoute(ctx *context.Context) {
}
}

func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMode int, opts *issue_indexer.SearchOptions) (*issues_model.IssueStats, error) {
func getUserIssueStats(ctx *context.Context, filterMode int, opts *issue_indexer.SearchOptions) (ret *issues_model.IssueStats, err error) {
ret = &issues_model.IssueStats{}
doerID := ctx.Doer.ID

opts = opts.Copy(func(o *issue_indexer.SearchOptions) {
// If the doer is the same as the context user, which means the doer is viewing his own dashboard,
// it's not enough to show the repos that the doer owns or has been explicitly granted access to,
// because the doer may create issues or be mentioned in any public repo.
// So we need search issues in all public repos.
o.AllPublic = doerID == ctxUser.ID
o.AssigneeID = nil
o.PosterID = nil
o.MentionID = nil
o.ReviewRequestedID = nil
o.ReviewedID = nil
})

var (
err error
ret = &issues_model.IssueStats{}
)

{
openClosedOpts := opts.Copy()
switch filterMode {
Expand Down
Loading
Loading