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 1 commit
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
47 changes: 47 additions & 0 deletions modules/templates/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,50 @@ func TestTemplateTruthy(t *testing.T) {
}
assert.True(t, truthyCount != 0 && truthyCount != len(cases))
}

func TestQueryBuild(t *testing.T) {
t.Run("construct", func(t *testing.T) {
assert.Equal(t, "", string(queryBuild()))
assert.Equal(t, "a=1&b=true", string(queryBuild("a", 1, "b", "true")))
assert.Equal(t, "?k=1", string(queryBuild("?", "k", 1)))
assert.Equal(t, "?a=b&k=1", string(queryBuild("?a=b", "k", 1)))
assert.Equal(t, "&k=1", string(queryBuild("&", "k", 1)))
assert.Equal(t, "&a=b&k=1", string(queryBuild("&a=b", "k", 1)))
})
t.Run("replace", func(t *testing.T) {
assert.Equal(t, "a=1&c=d&e=f", string(queryBuild(QueryString("a=b&c=d&e=f"), "a", 1)))
assert.Equal(t, "a=b&c=1&e=f", string(queryBuild(QueryString("a=b&c=d&e=f"), "c", 1)))
assert.Equal(t, "a=b&c=d&e=1", string(queryBuild(QueryString("a=b&c=d&e=f"), "e", 1)))
assert.Equal(t, "a=b&c=d&e=f&k=1", string(queryBuild(QueryString("a=b&c=d&e=f"), "k", 1)))
})
t.Run("replace-?", func(t *testing.T) {
assert.Equal(t, "?a=1&c=d&e=f", string(queryBuild(QueryString("?a=b&c=d&e=f"), "a", 1)))
assert.Equal(t, "?a=b&c=1&e=f", string(queryBuild(QueryString("?a=b&c=d&e=f"), "c", 1)))
assert.Equal(t, "?a=b&c=d&e=1", string(queryBuild(QueryString("?a=b&c=d&e=f"), "e", 1)))
assert.Equal(t, "?a=b&c=d&e=f&k=1", string(queryBuild(QueryString("?a=b&c=d&e=f"), "k", 1)))
})
t.Run("replace-&", func(t *testing.T) {
assert.Equal(t, "&a=1&c=d&e=f", string(queryBuild(QueryString("&a=b&c=d&e=f"), "a", 1)))
assert.Equal(t, "&a=b&c=1&e=f", string(queryBuild(QueryString("&a=b&c=d&e=f"), "c", 1)))
assert.Equal(t, "&a=b&c=d&e=1", string(queryBuild(QueryString("&a=b&c=d&e=f"), "e", 1)))
assert.Equal(t, "&a=b&c=d&e=f&k=1", string(queryBuild(QueryString("&a=b&c=d&e=f"), "k", 1)))
})
t.Run("delete", func(t *testing.T) {
assert.Equal(t, "c=d&e=f", string(queryBuild(QueryString("a=b&c=d&e=f"), "a", "")))
assert.Equal(t, "a=b&e=f", string(queryBuild(QueryString("a=b&c=d&e=f"), "c", "")))
assert.Equal(t, "a=b&c=d", string(queryBuild(QueryString("a=b&c=d&e=f"), "e", "")))
assert.Equal(t, "a=b&c=d&e=f", string(queryBuild(QueryString("a=b&c=d&e=f"), "k", "")))
})
t.Run("delete-?", func(t *testing.T) {
assert.Equal(t, "?c=d&e=f", string(queryBuild(QueryString("?a=b&c=d&e=f"), "a", "")))
assert.Equal(t, "?a=b&e=f", string(queryBuild(QueryString("?a=b&c=d&e=f"), "c", "")))
assert.Equal(t, "?a=b&c=d", string(queryBuild(QueryString("?a=b&c=d&e=f"), "e", "")))
assert.Equal(t, "?a=b&c=d&e=f", string(queryBuild(QueryString("?a=b&c=d&e=f"), "k", "")))
})
t.Run("delete-&", func(t *testing.T) {
assert.Equal(t, "&c=d&e=f", string(queryBuild(QueryString("&a=b&c=d&e=f"), "a", "")))
assert.Equal(t, "&a=b&e=f", string(queryBuild(QueryString("&a=b&c=d&e=f"), "c", "")))
assert.Equal(t, "&a=b&c=d", string(queryBuild(QueryString("&a=b&c=d&e=f"), "e", "")))
assert.Equal(t, "&a=b&c=d&e=f", string(queryBuild(QueryString("&a=b&c=d&e=f"), "k", "")))
})
}
68 changes: 36 additions & 32 deletions routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ 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/services/context"
feed_service "code.gitea.io/gitea/services/feed"
Expand Down Expand Up @@ -375,16 +376,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 +392,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 +437,12 @@ 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.
opts.PosterID, _ = strconv.ParseInt(ctx.FormString("poster"), 10, 64)
opts.AssigneeID, _ = strconv.ParseInt(ctx.FormString("assignee"), 10, 64)

isFuzzy := ctx.FormBool("fuzzy")

Expand Down Expand Up @@ -573,8 +573,21 @@ 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
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 +643,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 +658,13 @@ 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))
if opts.PosterID != 0 {
pager.AddParamString("poster", fmt.Sprint(opts.PosterID))
}
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 +789,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