dashboard: support filtering over multiple labels

For each label, allow only one value to be specified.
At the same time, allow multiple different labels (subsystem, origin,
prio, etc) be specified together.

3 files changed, 107 insertions, 26 deletions

diff --git a/dashboard/app/main.go b/dashboard/app/main.go
index d63856455..db19e2c62 100644
--- a/dashboard/app/main.go
+++ b/dashboard/app/main.go
@@ -82,15 +82,15 @@ type uiMainPage struct {
 
 type uiBugFilter struct {
 	Filter  *userBugFilter
-	DropURL func(string) string
+	DropURL func(string, string) string
 }
 
 func makeUIBugFilter(c context.Context, filter *userBugFilter) *uiBugFilter {
 	url := getCurrentURL(c)
 	return &uiBugFilter{
 		Filter: filter,
-		DropURL: func(name string) string {
-			return html.AmendURL(url, name, "")
+		DropURL: func(name, value string) string {
+			return html.DropParam(url, name, value)
 		},
 	}
 }
@@ -386,17 +386,20 @@ type uiJob struct {
 type userBugFilter struct {
 	Manager     string // show bugs that happened on the manager
 	OnlyManager string // show bugs that happened ONLY on the manager
-	Label       string // TODO: support multiple.
+	Labels      []string
 	NoSubsystem bool
 }
 
-func MakeBugFilter(r *http.Request) *userBugFilter {
+func MakeBugFilter(r *http.Request) (*userBugFilter, error) {
+	if err := r.ParseForm(); err != nil {
+		return nil, err
+	}
 	return &userBugFilter{
 		NoSubsystem: r.FormValue("no_subsystem") != "",
 		Manager:     r.FormValue("manager"),
 		OnlyManager: r.FormValue("only_manager"),
-		Label:       r.FormValue("label"),
-	}
+		Labels:      r.Form["label"],
+	}, nil
 }
 
 func (filter *userBugFilter) MatchManagerName(name string) bool {
@@ -427,9 +430,11 @@ func (filter *userBugFilter) MatchBug(bug *Bug) bool {
 	if filter.NoSubsystem && len(bug.LabelValues(SubsystemLabel)) > 0 {
 		return false
 	}
-	if filter.Label != "" {
-		label, value := splitLabel(filter.Label)
-		return bug.HasLabel(label, value)
+	for _, rawLabel := range filter.Labels {
+		label, value := splitLabel(rawLabel)
+		if !bug.HasLabel(label, value) {
+			return false
+		}
 	}
 	return true
 }
@@ -443,7 +448,7 @@ func (filter *userBugFilter) Any() bool {
 	if filter == nil {
 		return false
 	}
-	return filter.Label != "" || filter.OnlyManager != "" || filter.Manager != "" || filter.NoSubsystem
+	return len(filter.Labels) > 0 || filter.OnlyManager != "" || filter.Manager != "" || filter.NoSubsystem
 }
 
 // handleMain serves main page.
@@ -453,7 +458,10 @@ func handleMain(c context.Context, w http.ResponseWriter, r *http.Request) error
 		return err
 	}
 	accessLevel := accessLevel(c, r)
-	filter := MakeBugFilter(r)
+	filter, err := MakeBugFilter(r)
+	if err != nil {
+		return fmt.Errorf("%w: failed to parse URL parameters", ErrClientBadRequest)
+	}
 	managers, err := loadManagers(c, accessLevel, hdr.Namespace, filter)
 	if err != nil {
 		return err
@@ -516,10 +524,12 @@ func handleSubsystemPage(c context.Context, w http.ResponseWriter, r *http.Reque
 	}
 	groups, err := fetchNamespaceBugs(c, accessLevel(c, r),
 		hdr.Namespace, &userBugFilter{
-			Label: BugLabel{
-				Label: SubsystemLabel,
-				Value: subsystem.Name,
-			}.String(),
+			Labels: []string{
+				BugLabel{
+					Label: SubsystemLabel,
+					Value: subsystem.Name,
+				}.String(),
+			},
 		})
 	if err != nil {
 		return err
@@ -584,7 +594,10 @@ func handleTerminalBugList(c context.Context, w http.ResponseWriter, r *http.Req
 		return err
 	}
 	hdr.Subpage = typ.Subpage
-	typ.Filter = MakeBugFilter(r)
+	typ.Filter, err = MakeBugFilter(r)
+	if err != nil {
+		return fmt.Errorf("%w: failed to parse URL parameters", ErrClientBadRequest)
+	}
 	extraBugs := []*Bug{}
 	if typ.Status == BugStatusFixed {
 		// Mix in bugs that have pending fixes.
@@ -873,8 +886,9 @@ func makeBugLabelUI(c context.Context, bug *Bug, entry BugLabel) *uiBugLabel {
 	if !strings.HasPrefix(url, "/"+bug.Namespace) {
 		link = fmt.Sprintf("/%s", bug.Namespace)
 	}
-	link = html.AmendURL(link, "label", filterValue)
-
+	link = html.TransformURL(link, "label", func(oldLabels []string) []string {
+		return mergeLabelSet(oldLabels, entry.String())
+	})
 	ret := &uiBugLabel{
 		Name: filterValue,
 		Link: link,
@@ -892,6 +906,23 @@ func makeBugLabelUI(c context.Context, bug *Bug, entry BugLabel) *uiBugLabel {
 	return ret
 }
 
+func mergeLabelSet(oldLabels []string, newLabel string) []string {
+	// Leave only one label for each type.
+	labelsMap := map[BugLabelType]string{}
+	for _, rawLabel := range append(oldLabels, newLabel) {
+		label, value := splitLabel(rawLabel)
+		labelsMap[label] = value
+	}
+	var ret []string
+	for label, value := range labelsMap {
+		ret = append(ret, BugLabel{
+			Label: label,
+			Value: value,
+		}.String())
+	}
+	return ret
+}
+
 func getBugDiscussionsUI(c context.Context, bug *Bug) ([]*uiBugDiscussion, error) {
 	// TODO: also include dup bug discussions.
 	// TODO: limit the number of DiscussionReminder type entries, e.g. all with
@@ -1386,8 +1417,9 @@ func fetchTerminalBugs(c context.Context, accessLevel AccessLevel,
 
 func applyBugFilter(query *db.Query, filter *userBugFilter) *db.Query {
 	manager := filter.ManagerName()
-	label, value := splitLabel(filter.Label)
-	if label != EmptyLabel {
+	if len(filter.Labels) > 0 {
+		// Take just the first one.
+		label, value := splitLabel(filter.Labels[0])
 		query = query.Filter("Labels.Label=", string(label))
 		query = query.Filter("Labels.Value=", value)
 	} else if manager != "" {
diff --git a/dashboard/app/main_test.go b/dashboard/app/main_test.go
index be1b3e847..fff94d7f9 100644
--- a/dashboard/app/main_test.go
+++ b/dashboard/app/main_test.go
@@ -247,3 +247,51 @@ func TestSubsystemPage(t *testing.T) {
 	assert.Contains(t, string(reply), crash1.Title)
 	assert.NotContains(t, string(reply), crash2.Title)
 }
+
+func TestMultiLabelFilter(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	client := c.makeClient(clientPublicEmail, keyPublicEmail, true)
+	mailingList := config.Namespaces["access-public-email"].Reporting[0].Config.(*EmailConfig).Email
+
+	build1 := testBuild(1)
+	build1.Manager = "manager-name-123"
+	client.UploadBuild(build1)
+
+	crash1 := testCrash(build1, 1)
+	crash1.GuiltyFiles = []string{"a.c"}
+	crash1.Title = "crash-with-subsystem-A"
+	client.ReportCrash(crash1)
+	c.pollEmailBug()
+
+	crash2 := testCrash(build1, 2)
+	crash2.GuiltyFiles = []string{"a.c"}
+	crash2.Title = "prio-crash-subsystem-A"
+	client.ReportCrash(crash2)
+
+	c.incomingEmail(c.pollEmailBug().Sender, "#syz set prio: low\n",
+		EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList}))
+
+	// The normal main page.
+	reply, err := c.AuthGET(AccessAdmin, "/access-public-email")
+	c.expectOK(err)
+	assert.Contains(t, string(reply), build1.Manager)
+	assert.NotContains(t, string(reply), "Applied filters")
+
+	reply, err = c.AuthGET(AccessAdmin, "/access-public-email?label=subsystems:subsystemA")
+	c.expectOK(err)
+	assert.Contains(t, string(reply), "Applied filters") // we're seeing a prompt to disable the filter
+	assert.Contains(t, string(reply), crash1.Title)
+	assert.Contains(t, string(reply), crash2.Title)
+
+	// Test filters together.
+	reply, err = c.AuthGET(AccessAdmin, "/access-public-email?label=subsystems:subsystemA&&label=prio:low")
+	c.expectOK(err)
+	assert.NotContains(t, string(reply), crash1.Title)
+	assert.Contains(t, string(reply), crash2.Title)
+
+	// Ensure we provide links that drop labels.
+	assert.NotContains(t, string(reply), "/access-public-email?label=subsystems:subsystemA\"")
+	assert.NotContains(t, string(reply), "/access-public-email?label=prop:low\"")
+}
diff --git a/dashboard/app/templates.html b/dashboard/app/templates.html
index 044f0fd42..001eec25e 100644
--- a/dashboard/app/templates.html
+++ b/dashboard/app/templates.html
@@ -94,16 +94,17 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the
 {{if .Filter.Any}}
 	<b>Applied filters: </b>
 	{{if .Filter.Manager}}
-		Manager={{.Filter.Manager}} ({{link (call .DropURL "manager") "drop"}})
+		Manager={{.Filter.Manager}} ({{link (call .DropURL "manager" "") "drop"}})
 	{{end}}
 	{{if .Filter.OnlyManager}}
-		Only Manager={{.Filter.OnlyManager}} ({{link (call .DropURL "only_manager") "drop"}})
+		Only Manager={{.Filter.OnlyManager}} ({{link (call .DropURL "only_manager" "") "drop"}})
 	{{end}}
 	{{if .Filter.NoSubsystem}}
-		NoSubsystem={{.Filter.NoSubsystem}} ({{link (call .DropURL "no_subsystem") "drop"}})
+		NoSubsystem={{.Filter.NoSubsystem}} ({{link (call .DropURL "no_subsystem" "") "drop"}})
 	{{end}}
-	{{if .Filter.Label}}
-		Label={{.Filter.Label}} ({{link (call .DropURL "label") "drop"}})
+	{{$drop := .DropURL}}
+	{{range .Filter.Labels}}
+		Label={{.}} ({{link (call $drop "label" .) "drop"}})
 	{{end}}
 	<br>
 {{end}}