Skip to content

Commit

Permalink
Refactor issue list (#32755)
Browse files Browse the repository at this point in the history 
1. add backend support for filtering "poster" and "assignee"
    * due to the limits, there is no frontend support at the moment
2. rewrite TS code without jquery, now there are 14 jQuery files left:
  • Loading branch information
@wxiaoguang
wxiaoguang authored Dec 8, 2024
1 parent 9d08d3f commit 23471e1
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 155 deletions.
66 changes: 66 additions & 0 deletions modules/templates/helper.go
View file
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
View file
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
View file
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
View file
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

0 comments on commit 23471e1

Please sign in to comment.