Skip to content

Commit 90d20be

Browse files
wxiaoguangwxiaoguang
authored
Refactor issue filter (labels, poster, assignee) (#32771)
Rewrite a lot of legacy strange code, remove duplicate code, remove jquery, and make these filters reusable. Let's forget the old code, new code affects: * issue list open/close switch * issue list filter (label, author, assignee) * milestone list open/close switch * milestone issue list filter (label, author, assignee) * project view (label, assignee)
1 parent 1b069dc commit 90d20be

18 files changed

+293
-320
lines changed

‎modules/templates/helper.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func NewFuncMap() template.FuncMap {
4242
"HTMLFormat": htmlutil.HTMLFormat,
4343
"HTMLEscape": htmlEscape,
4444
"QueryEscape": queryEscape,
45-
"QueryBuild": queryBuild,
45+
"QueryBuild": QueryBuild,
4646
"JSEscape": jsEscapeSafe,
4747
"SanitizeHTML": SanitizeHTML,
4848
"URLJoin": util.URLJoin,
@@ -294,24 +294,27 @@ func timeEstimateString(timeSec any) string {
294294
return util.TimeEstimateString(v)
295295
}
296296

297-
func queryBuild(a ...any) template.URL {
297+
// QueryBuild builds a query string from a list of key-value pairs.
298+
// It omits the nil and empty strings, but it doesn't omit other zero values,
299+
// because the zero value of number types may have a meaning.
300+
func QueryBuild(a ...any) template.URL {
298301
var s string
299302
if len(a)%2 == 1 {
300303
if v, ok := a[0].(string); ok {
301304
if v == "" || (v[0] != '?' && v[0] != '&') {
302-
panic("queryBuild: invalid argument")
305+
panic("QueryBuild: invalid argument")
303306
}
304307
s = v
305308
} else if v, ok := a[0].(template.URL); ok {
306309
s = string(v)
307310
} else {
308-
panic("queryBuild: invalid argument")
311+
panic("QueryBuild: invalid argument")
309312
}
310313
}
311314
for i := len(a) % 2; i < len(a); i += 2 {
312315
k, ok := a[i].(string)
313316
if !ok {
314-
panic("queryBuild: invalid argument")
317+
panic("QueryBuild: invalid argument")
315318
}
316319
var v string
317320
if va, ok := a[i+1].(string); ok {

‎options/locale/locale_en-US.ini

+3-2
Original file line numberDiff line numberDiff line change
@@ -1109,7 +1109,7 @@ delete_preexisting_success = Deleted unadopted files in %s
11091109
blame_prior = View blame prior to this change
11101110
blame.ignore_revs = Ignoring revisions in <a href="%s">.git-blame-ignore-revs</a>. Click <a href="%s">here to bypass</a> and see the normal blame view.
11111111
blame.ignore_revs.failed = Failed to ignore revisions in <a href="%s">.git-blame-ignore-revs</a>.
1112-
author_search_tooltip = Shows a maximum of 30 users
1112+
user_search_tooltip = Shows a maximum of 30 users
11131113
11141114
tree_path_not_found_commit = Path %[1]s doesn't exist in commit %[2]s
11151115
tree_path_not_found_branch = Path %[1]s doesn't exist in branch %[2]s
@@ -1529,7 +1529,8 @@ issues.filter_assignee = Assignee
15291529
issues.filter_assginee_no_select = All assignees
15301530
issues.filter_assginee_no_assignee = No assignee
15311531
issues.filter_poster = Author
1532-
issues.filter_poster_no_select = All authors
1532+
issues.filter_user_placeholder = Search users
1533+
issues.filter_user_no_select = All users
15331534
issues.filter_type = Type
15341535
issues.filter_type.all_issues = All issues
15351536
issues.filter_type.assigned_to_you = Assigned to you

‎routers/web/org/projects.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,7 @@ func ViewProject(ctx *context.Context) {
339339
// 0 means issues with no label
340340
// blank means labels will not be filtered for issues
341341
selectLabels := ctx.FormString("labels")
342-
if selectLabels == "" {
343-
ctx.Data["AllLabels"] = true
344-
} else if selectLabels == "0" {
345-
ctx.Data["NoLabel"] = true
346-
}
347-
if len(selectLabels) > 0 {
342+
if selectLabels != "" {
348343
labelIDs, err = base.StringsToInt64s(strings.Split(selectLabels, ","))
349344
if err != nil {
350345
ctx.Flash.Error(ctx.Tr("invalid_data", selectLabels), true)

‎routers/web/repo/issue_list.go

+7-22
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"bytes"
88
"fmt"
99
"net/http"
10-
"net/url"
1110
"strconv"
1211
"strings"
1312

@@ -531,12 +530,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
531530
// 0 means issues with no label
532531
// blank means labels will not be filtered for issues
533532
selectLabels := ctx.FormString("labels")
534-
if selectLabels == "" {
535-
ctx.Data["AllLabels"] = true
536-
} else if selectLabels == "0" {
537-
ctx.Data["NoLabel"] = true
538-
}
539-
if len(selectLabels) > 0 {
533+
if selectLabels != "" {
540534
labelIDs, err = base.StringsToInt64s(strings.Split(selectLabels, ","))
541535
if err != nil {
542536
ctx.Flash.Error(ctx.Tr("invalid_data", selectLabels), true)
@@ -616,8 +610,6 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
616610
ctx.Data["TotalTrackedTime"] = totalTrackedTime
617611
}
618612

619-
archived := ctx.FormBool("archived")
620-
621613
page := ctx.FormInt("page")
622614
if page <= 1 {
623615
page = 1
@@ -792,28 +784,21 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
792784
return
793785
}
794786

787+
showArchivedLabels := ctx.FormBool("archived_labels")
788+
ctx.Data["ShowArchivedLabels"] = showArchivedLabels
795789
ctx.Data["PinnedIssues"] = pinned
796790
ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.Doer.IsAdmin)
797791
ctx.Data["IssueStats"] = issueStats
798792
ctx.Data["OpenCount"] = issueStats.OpenCount
799793
ctx.Data["ClosedCount"] = issueStats.ClosedCount
800-
linkStr := "%s?q=%s&type=%s&sort=%s&state=%s&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%v&archived=%t"
801-
ctx.Data["AllStatesLink"] = fmt.Sprintf(linkStr, ctx.Link,
802-
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "all", url.QueryEscape(selectLabels),
803-
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
804-
ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Link,
805-
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "open", url.QueryEscape(selectLabels),
806-
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
807-
ctx.Data["ClosedLink"] = fmt.Sprintf(linkStr, ctx.Link,
808-
url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "closed", url.QueryEscape(selectLabels),
809-
milestoneID, projectID, assigneeID, url.QueryEscape(posterUsername), archived)
810794
ctx.Data["SelLabelIDs"] = labelIDs
811795
ctx.Data["SelectLabels"] = selectLabels
812796
ctx.Data["ViewType"] = viewType
813797
ctx.Data["SortType"] = sortType
814798
ctx.Data["MilestoneID"] = milestoneID
815799
ctx.Data["ProjectID"] = projectID
816800
ctx.Data["AssigneeID"] = assigneeID
801+
ctx.Data["PosterUserID"] = posterUserID
817802
ctx.Data["PosterUsername"] = posterUsername
818803
ctx.Data["Keyword"] = keyword
819804
ctx.Data["IsShowClosed"] = isShowClosed
@@ -825,7 +810,6 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
825810
default:
826811
ctx.Data["State"] = "open"
827812
}
828-
ctx.Data["ShowArchivedLabels"] = archived
829813

830814
pager.AddParamString("q", keyword)
831815
pager.AddParamString("type", viewType)
@@ -836,8 +820,9 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
836820
pager.AddParamString("project", fmt.Sprint(projectID))
837821
pager.AddParamString("assignee", fmt.Sprint(assigneeID))
838822
pager.AddParamString("poster", posterUsername)
839-
pager.AddParamString("archived", fmt.Sprint(archived))
840-
823+
if showArchivedLabels {
824+
pager.AddParamString("archived_labels", "true")
825+
}
841826
ctx.Data["Page"] = pager
842827
}
843828

‎routers/web/repo/milestone.go

-6
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,6 @@ func Milestones(ctx *context.Context) {
6666
}
6767
ctx.Data["OpenCount"] = stats.OpenCount
6868
ctx.Data["ClosedCount"] = stats.ClosedCount
69-
linkStr := "%s/milestones?state=%s&q=%s&sort=%s"
70-
ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Repo.RepoLink, "open",
71-
url.QueryEscape(keyword), url.QueryEscape(sortType))
72-
ctx.Data["ClosedLink"] = fmt.Sprintf(linkStr, ctx.Repo.RepoLink, "closed",
73-
url.QueryEscape(keyword), url.QueryEscape(sortType))
74-
7569
if ctx.Repo.Repository.IsTimetrackerEnabled(ctx) {
7670
if err := issues_model.MilestoneList(miles).LoadTotalTrackedTimes(ctx); err != nil {
7771
ctx.ServerError("LoadTotalTrackedTimes", err)

‎routers/web/repo/projects.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -312,12 +312,7 @@ func ViewProject(ctx *context.Context) {
312312
// 0 means issues with no label
313313
// blank means labels will not be filtered for issues
314314
selectLabels := ctx.FormString("labels")
315-
if selectLabels == "" {
316-
ctx.Data["AllLabels"] = true
317-
} else if selectLabels == "0" {
318-
ctx.Data["NoLabel"] = true
319-
}
320-
if len(selectLabels) > 0 {
315+
if selectLabels != "" {
321316
labelIDs, err = base.StringsToInt64s(strings.Split(selectLabels, ","))
322317
if err != nil {
323318
ctx.Flash.Error(ctx.Tr("invalid_data", selectLabels), true)

‎templates/projects/view.tmpl

+12-72
Original file line numberDiff line numberDiff line change
@@ -5,79 +5,19 @@
55
<h2 class="tw-mb-0 tw-flex-1 tw-break-anywhere">{{.Project.Title}}</h2>
66
<div class="project-toolbar-right">
77
<div class="ui secondary filter menu labels">
8-
<!-- Label -->
9-
<div class="ui {{if not .Labels}}disabled{{end}} dropdown jump item label-filter">
10-
<span class="text">
11-
{{ctx.Locale.Tr "repo.issues.filter_label"}}
12-
</span>
13-
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
14-
<div class="menu">
15-
<div class="ui icon search input">
16-
<i class="icon">{{svg "octicon-search" 16}}</i>
17-
<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_label"}}">
18-
</div>
19-
<div class="ui checkbox compact archived-label-filter">
20-
<input name="archived" type="checkbox"
21-
id="archived-filter-checkbox"
22-
{{if .ShowArchivedLabels}}checked{{end}}
23-
>
24-
<label for="archived-filter-checkbox">
25-
{{ctx.Locale.Tr "repo.issues.label_archived_filter"}}
26-
<i class="tw-ml-1" data-tooltip-content={{ctx.Locale.Tr "repo.issues.label_archive_tooltip"}}>
27-
{{svg "octicon-info"}}
28-
</i>
29-
</label>
30-
</div>
31-
<span class="info">{{ctx.Locale.Tr "repo.issues.filter_label_exclude"}}</span>
32-
<div class="divider"></div>
33-
<a class="{{if .AllLabels}}active selected {{end}}item" href="?assignee={{$.AssigneeID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}}</a>
34-
<a class="{{if .NoLabel}}active selected {{end}}item" href="?assignee={{$.AssigneeID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
35-
{{$previousExclusiveScope := "_no_scope"}}
36-
{{range .Labels}}
37-
{{$exclusiveScope := .ExclusiveScope}}
38-
{{if and (ne $previousExclusiveScope $exclusiveScope)}}
39-
<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
40-
{{end}}
41-
{{$previousExclusiveScope = $exclusiveScope}}
42-
<a class="item label-filter-item tw-flex tw-items-center" data-label-id="{{.ID}}" data-scope="{{.ExclusiveScope}}" {{if .IsArchived}}data-is-archived{{end}}
43-
href="?labels={{.QueryString}}&assignee={{$.AssigneeID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">
44-
{{if .IsExcluded}}
45-
{{svg "octicon-circle-slash"}}
46-
{{else if .IsSelected}}
47-
{{if $exclusiveScope}}
48-
{{svg "octicon-dot-fill"}}
49-
{{else}}
50-
{{svg "octicon-check"}}
51-
{{end}}
52-
{{end}}
53-
{{ctx.RenderUtils.RenderLabel .}}
54-
<p class="tw-ml-auto">{{template "repo/issue/labels/label_archived" .}}</p>
55-
</a>
56-
{{end}}
57-
</div>
58-
</div>
8+
{{$queryLink := QueryBuild "?" "labels" .SelectLabels "assignee" $.AssigneeID "archived_labels" (Iif $.ShowArchivedLabels "true")}}
599

60-
<!-- Assignee -->
61-
<div class="ui {{if not .Assignees}}disabled{{end}} dropdown jump item">
62-
<span class="text">
63-
{{ctx.Locale.Tr "repo.issues.filter_assignee"}}
64-
</span>
65-
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
66-
<div class="menu">
67-
<div class="ui icon search input">
68-
<i class="icon">{{svg "octicon-search" 16}}</i>
69-
<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_assignee"}}">
70-
</div>
71-
<a class="{{if not .AssigneeID}}active selected {{end}}item" href="?labels={{.SelectLabels}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">{{ctx.Locale.Tr "repo.issues.filter_assginee_no_select"}}</a>
72-
<a class="{{if eq .AssigneeID -1}}active selected {{end}}item" href="?labels={{.SelectLabels}}&assignee=-1{{if $.ShowArchivedLabels}}&archived=true{{end}}">{{ctx.Locale.Tr "repo.issues.filter_assginee_no_assignee"}}</a>
73-
<div class="divider"></div>
74-
{{range .Assignees}}
75-
<a class="{{if eq $.AssigneeID .ID}}active selected{{end}} item tw-flex" href="?labels={{$.SelectLabels}}&assignee={{.ID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">
76-
{{ctx.AvatarUtils.Avatar . 20}}{{template "repo/search_name" .}}
77-
</a>
78-
{{end}}
79-
</div>
80-
</div>
10+
{{template "repo/issue/filter_item_label" dict "Labels" .Labels "QueryLink" $queryLink "SupportArchivedLabel" true}}
11+
12+
{{template "repo/issue/filter_item_user_assign" dict
13+
"QueryParamKey" "assignee"
14+
"QueryLink" $queryLink
15+
"UserSearchList" $.Assignees
16+
"SelectedUserId" $.AssigneeID
17+
"TextFilterTitle" (ctx.Locale.Tr "repo.issues.filter_assignee")
18+
"TextZeroValue" (ctx.Locale.Tr "repo.issues.filter_assginee_no_select")
19+
"TextNegativeOne" (ctx.Locale.Tr "repo.issues.filter_assginee_no_assignee")
20+
}}
8121
</div>
8222
</div>
8323
{{if $canWriteProject}}

‎templates/repo/issue/filter_item_label.tmpl

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
{{/*
2+
* "labels" from query string (needed by JS)
3+
* QueryLink
4+
* Labels
5+
* SupportArchivedLabel, if true, then it needs "archived_labels" from query string
6+
*/}}
7+
{{$queryLink := .QueryLink}}
8+
<div class="item ui dropdown jump {{if not .Labels}}disabled{{end}} label-filter">
9+
<span class="text">{{ctx.Locale.Tr "repo.issues.filter_label"}}</span>
10+
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
11+
<div class="menu flex-items-menu">
12+
<div class="ui icon search input">
13+
<i class="icon">{{svg "octicon-search" 16}}</i>
14+
<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_label"}}">
15+
</div>
16+
{{if .SupportArchivedLabel}}{{/* this checkbox has a hard dependency with the "labels" and "archived_label" query parameter */}}
17+
<label class="label-filter-archived-toggle flex-text-block">
18+
<input type="checkbox"> {{ctx.Locale.Tr "repo.issues.label_archived_filter"}}
19+
<span data-tooltip-content={{ctx.Locale.Tr "repo.issues.label_archive_tooltip"}}>{{svg "octicon-info"}}</span>
20+
</label>
21+
{{end}}
22+
<span class="info">{{ctx.Locale.Tr "repo.issues.filter_label_exclude"}}</span>
23+
<div class="divider"></div>
24+
<a class="item label-filter-query-default" href="{{QueryBuild $queryLink "labels" NIL}}">{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}}</a>
25+
<a class="item label-filter-query-not-set" href="{{QueryBuild $queryLink "labels" 0}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
26+
{{$previousExclusiveScope := "_no_scope"}}
27+
{{range .Labels}}
28+
{{$exclusiveScope := .ExclusiveScope}}
29+
{{if and (ne $previousExclusiveScope $exclusiveScope)}}
30+
<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
31+
{{end}}
32+
{{$previousExclusiveScope = $exclusiveScope}}
33+
<a class="item label-filter-query-item" data-label-id="{{.ID}}" data-scope="{{.ExclusiveScope}}" {{if .IsArchived}}data-is-archived{{end}}
34+
href="{{QueryBuild $queryLink "labels" .QueryString}}">
35+
{{if .IsExcluded}}
36+
{{svg "octicon-circle-slash"}}
37+
{{else if .IsSelected}}
38+
{{Iif $exclusiveScope (svg "octicon-dot-fill") (svg "octicon-check")}}
39+
{{end}}
40+
{{ctx.RenderUtils.RenderLabel .}}
41+
<p class="tw-ml-auto">{{template "repo/issue/labels/label_archived" .}}</p>
42+
</a>
43+
{{end}}
44+
</div>
45+
</div>

‎templates/repo/issue/filter_item_user_assign.tmpl

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
{{/* This is a user list for filter, the data is provided by a local variable assignment
2+
* QueryParamKey: eg: "poster", "assignee"
3+
* QueryLink
4+
* UserSearchList
5+
* SelectedUserId: 0 or empty means default, -1 means "no user is set"
6+
* TextFilterTitle
7+
* TextZeroValue: the text for "all issues"
8+
* TextNegativeOne: the text for "issues with no assignee"
9+
*/}}
10+
{{$queryLink := .QueryLink}}
11+
<div class="item ui dropdown jump {{if not .UserSearchList}}disabled{{end}}">
12+
{{$.TextFilterTitle}} {{svg "octicon-triangle-down" 14 "dropdown icon"}}
13+
<div class="menu">
14+
<div class="ui icon search input">
15+
<i class="icon">{{svg "octicon-search" 16}}</i>
16+
<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_user_placeholder"}}">
17+
</div>
18+
{{if $.TextZeroValue}}
19+
<a class="item {{if not .SelectedUserId}}selected{{end}}" href="{{QueryBuild $queryLink $.QueryParamKey NIL}}">{{$.TextZeroValue}}</a>
20+
{{end}}
21+
{{if $.TextNegativeOne}}
22+
<a class="item {{if eq .SelectedUserId -1}}selected{{end}}" href="{{QueryBuild $queryLink $.QueryParamKey -1}}">{{$.TextNegativeOne}}</a>
23+
{{end}}
24+
<div class="divider"></div>
25+
{{range .UserSearchList}}
26+
<a class="item {{if eq $.SelectedUserId .ID}}selected{{end}}" href="{{QueryBuild $queryLink $.QueryParamKey .ID}}">
27+
{{ctx.AvatarUtils.Avatar . 20}}{{template "repo/search_name" .}}
28+
</a>
29+
{{end}}
30+
</div>
31+
</div>

‎templates/repo/issue/filter_item_user_fetch.tmpl

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{{/* This is a user list for filter, the data is provided by a remote "fetch" request
2+
* QueryParamKey: eg: "poster", "assignee"
3+
* QueryLink
4+
* UserSearchUrl
5+
* SelectedUserId
6+
* TextFilterTitle
7+
*/}}
8+
{{$queryLink := .QueryLink}}
9+
<div class="item ui dropdown custom user-remote-search" data-tooltip-content="{{ctx.Locale.Tr "repo.user_search_tooltip"}}"
10+
data-search-url="{{$.UserSearchUrl}}"
11+
data-selected-user-id="{{$.SelectedUserId}}"
12+
data-action-jump-url="{{QueryBuild $queryLink $.QueryParamKey NIL}}&{{$.QueryParamKey}}={username}"
13+
>
14+
{{$.TextFilterTitle}} {{svg "octicon-triangle-down" 14 "dropdown icon"}}
15+
<div class="menu">
16+
<div class="ui icon search input">
17+
<i class="icon">{{svg "octicon-search" 16}}</i>
18+
<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_user_placeholder"}}">
19+
</div>
20+
<a class="item" data-value="">{{ctx.Locale.Tr "repo.issues.filter_user_no_select"}}</a>
21+
<a class="item item-from-input tw-hidden"></a>
22+
</div>
23+
</div>

0 commit comments

Comments
 (0)