From 0d10b62e082578639ad515ccc7cde07e09fcce4e Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 31 Jan 2018 20:41:41 +0800 Subject: [PATCH 1/7] Add the ability to use multiple labels as filters (#3430) --- models/issue.go | 24 ++++++++++++++++++++++-- models/issue_label.go | 21 +++++++++++++++++++++ routers/repo/issue_label.go | 19 +++++++++++++++++++ routers/routes/routes.go | 2 +- templates/repo/issue/list.tmpl | 2 +- 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/models/issue.go b/models/issue.go index d10c521db6d8f..b0ef837c2a8f2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1140,10 +1140,20 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) error { if err != nil { return err } - if len(labelIDs) > 0 { + if len(labelIDs) == 1 { sess. Join("INNER", "issue_label", "issue.id = issue_label.issue_id"). In("issue_label.label_id", labelIDs) + } else if len(labelIDs) > 1{ + cond, args, _ := builder.ToSQL(builder.In("issue_label.label_id", labelIDs)) + sess. + Where(fmt.Sprintf(`issue.id IN ( + SELECT issue_label.issue_id + FROM issue_label + WHERE %s + GROUP BY issue_label.issue_id + HAVING COUNT(issue_label.label_id) = %d + )`, cond, len(labelIDs)), args...) } } return nil @@ -1322,9 +1332,19 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { labelIDs, err := base.StringsToInt64s(strings.Split(opts.Labels, ",")) if err != nil { log.Warn("Malformed Labels argument: %s", opts.Labels) - } else if len(labelIDs) > 0 { + } else if len(labelIDs) == 1 { sess.Join("INNER", "issue_label", "issue.id = issue_label.issue_id"). In("issue_label.label_id", labelIDs) + } else if len(labelIDs) > 1 { + cond, args, _ := builder.ToSQL(builder.In("issue_label.label_id", labelIDs)) + sess. + Where(fmt.Sprintf(`issue.id IN ( + SELECT issue_label.issue_id + FROM issue_label + WHERE %s + GROUP BY issue_label.issue_id + HAVING COUNT(issue_label.label_id) = %d + )`, cond, len(labelIDs)), args...) } } diff --git a/models/issue_label.go b/models/issue_label.go index 6e48b4633bd31..94670ab7153f9 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -60,6 +60,8 @@ type Label struct { NumClosedIssues int NumOpenIssues int `xorm:"-"` IsChecked bool `xorm:"-"` + QueryString string + IsSelected bool } // APIFormat converts a Label to the api.Label format @@ -76,6 +78,25 @@ func (label *Label) CalOpenIssues() { label.NumOpenIssues = label.NumIssues - label.NumClosedIssues } +// CalQueryString calculates query string in issue/pulls list +func (label *Label) CalQueryString(query []string) { + var labelQuerySlice []string + labelSelected := false + labelID := fmt.Sprint(label.ID) + for _, s := range query { + if s == labelID { + labelSelected = true + } else { + labelQuerySlice = append(labelQuerySlice, s) + } + } + if !labelSelected { + labelQuerySlice = append(labelQuerySlice, labelID) + } + label.IsSelected = labelSelected + label.QueryString = strings.Join(labelQuerySlice, ",") +} + // ForegroundColor calculates the text color for labels based // on their background color. func (label *Label) ForegroundColor() template.CSS { diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index a197256be86db..9247acbb00871 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -5,6 +5,8 @@ package repo import ( + "strings" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/base" @@ -69,6 +71,23 @@ func RetrieveLabels(ctx *context.Context) { ctx.Data["SortType"] = ctx.Query("sort") } +// RetreveLabelsAndCalQueryString calculate query string when filtering issues/pulls +func RetreveLabelsAndCalQueryString(ctx *context.Context) { + labels, err := models.GetLabelsByRepoID(ctx.Repo.Repository.ID, ctx.Query("sort")) + if err != nil { + ctx.ServerError("RetreveLabelsAndCalQueryString.GetLabels", err) + return + } + selectLabelsSlice := strings.Split(ctx.Query("labels"), ",") + for _, l := range labels { + l.CalOpenIssues() + l.CalQueryString(selectLabelsSlice) + } + ctx.Data["Labels"] = labels + ctx.Data["NumLabels"] = len(labels) + ctx.Data["SortType"] = ctx.Query("sort") +} + // NewLabel create new label for repository func NewLabel(ctx *context.Context, form auth.CreateLabelForm) { ctx.Data["Title"] = ctx.Tr("repo.labels") diff --git a/routers/routes/routes.go b/routers/routes/routes.go index e51bfb946ab72..607845d8a9e71 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -585,7 +585,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/:username/:reponame", func() { m.Group("", func() { - m.Get("/^:type(issues|pulls)$", repo.RetrieveLabels, repo.Issues) + m.Get("/^:type(issues|pulls)$", repo.RetreveLabelsAndCalQueryString, repo.Issues) m.Get("/^:type(issues|pulls)$/:index", repo.ViewIssue) m.Get("/labels/", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.RetrieveLabels, repo.Labels) m.Get("/milestones", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.Milestones) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 642458ff1e43c..117b7e7111827 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -42,7 +42,7 @@
{{.i18n.Tr "repo.issues.filter_label_no_select"}} {{range .Labels}} - {{.Name}} + {{.Name}} {{end}}
From e33b02e4e4a4b1e83d270083d001d9f00e0ccefa Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 31 Jan 2018 20:56:37 +0800 Subject: [PATCH 2/7] to fix a bug when select only one label --- models/issue_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_label.go b/models/issue_label.go index 94670ab7153f9..8a0ba23b0745c 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -86,7 +86,7 @@ func (label *Label) CalQueryString(query []string) { for _, s := range query { if s == labelID { labelSelected = true - } else { + } else if s != "" { labelQuerySlice = append(labelQuerySlice, s) } } From 01252ba2fd41203c21721698b2dbfdd5984e2a41 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 31 Jan 2018 22:01:08 +0800 Subject: [PATCH 3/7] fix format --- models/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue.go b/models/issue.go index b0ef837c2a8f2..01252a6d24c77 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1144,7 +1144,7 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) error { sess. Join("INNER", "issue_label", "issue.id = issue_label.issue_id"). In("issue_label.label_id", labelIDs) - } else if len(labelIDs) > 1{ + } else if len(labelIDs) > 1 { cond, args, _ := builder.ToSQL(builder.In("issue_label.label_id", labelIDs)) sess. Where(fmt.Sprintf(`issue.id IN ( From 5d800446e5e36b55b640d1da2b07fed72f480d9f Mon Sep 17 00:00:00 2001 From: Song Guo Date: Sat, 3 Feb 2018 13:47:33 +0800 Subject: [PATCH 4/7] rename some variables and functions --- models/issue_label.go | 6 +++--- routers/repo/issue_label.go | 10 +++++----- routers/routes/routes.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/models/issue_label.go b/models/issue_label.go index 8a0ba23b0745c..216c5e7218c54 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -78,12 +78,12 @@ func (label *Label) CalOpenIssues() { label.NumOpenIssues = label.NumIssues - label.NumClosedIssues } -// CalQueryString calculates query string in issue/pulls list -func (label *Label) CalQueryString(query []string) { +// LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked +func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []string) { var labelQuerySlice []string labelSelected := false labelID := fmt.Sprint(label.ID) - for _, s := range query { + for _, s := range currentSelectedLabels { if s == labelID { labelSelected = true } else if s != "" { diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index 9247acbb00871..09280448fc635 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -71,17 +71,17 @@ func RetrieveLabels(ctx *context.Context) { ctx.Data["SortType"] = ctx.Query("sort") } -// RetreveLabelsAndCalQueryString calculate query string when filtering issues/pulls -func RetreveLabelsAndCalQueryString(ctx *context.Context) { +// RetrieveLabelsAndLoadSelectedLabels calculate query string when filtering issues/pulls +func RetrieveLabelsAndLoadSelectedLabels(ctx *context.Context) { labels, err := models.GetLabelsByRepoID(ctx.Repo.Repository.ID, ctx.Query("sort")) if err != nil { - ctx.ServerError("RetreveLabelsAndCalQueryString.GetLabels", err) + ctx.ServerError("RetrieveLabelsAndLoadSelectedLabels.GetLabels", err) return } - selectLabelsSlice := strings.Split(ctx.Query("labels"), ",") + selectLabels := strings.Split(ctx.Query("labels"), ",") for _, l := range labels { l.CalOpenIssues() - l.CalQueryString(selectLabelsSlice) + l.LoadSelectedLabelsAfterClick(selectLabels) } ctx.Data["Labels"] = labels ctx.Data["NumLabels"] = len(labels) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 607845d8a9e71..212373a5d0f63 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -585,7 +585,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/:username/:reponame", func() { m.Group("", func() { - m.Get("/^:type(issues|pulls)$", repo.RetreveLabelsAndCalQueryString, repo.Issues) + m.Get("/^:type(issues|pulls)$", repo.RetrieveLabelsAndLoadSelectedLabels, repo.Issues) m.Get("/^:type(issues|pulls)$/:index", repo.ViewIssue) m.Get("/labels/", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.RetrieveLabels, repo.Labels) m.Get("/milestones", context.CheckAnyUnit(models.UnitTypeIssues, models.UnitTypePullRequests), repo.Milestones) From ccf70563b16b85e539ca196139a39b5ea6280b0b Mon Sep 17 00:00:00 2001 From: Song Guo Date: Mon, 5 Feb 2018 10:59:07 +0800 Subject: [PATCH 5/7] update tests to match expected value when multiple labels selected, only issues with all the selected labels will be shown --- models/fixtures/issue_label.yml | 5 +++++ models/issue_test.go | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/models/fixtures/issue_label.yml b/models/fixtures/issue_label.yml index 49d5a95d020f3..1f0a91ad998b5 100644 --- a/models/fixtures/issue_label.yml +++ b/models/fixtures/issue_label.yml @@ -12,3 +12,8 @@ id: 3 issue_id: 2 label_id: 1 + +- + id: 4 + issue_id: 1 + label_id: 2 diff --git a/models/issue_test.go b/models/issue_test.go index 851fe684fbc79..b0855c4504f48 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -191,13 +191,21 @@ func TestIssues(t *testing.T) { }, []int64{1, 2, 3, 5}, }, + { + IssuesOptions{ + Labels: "1", + Page: 1, + PageSize: 4, + }, + []int64{2, 1}, + }, { IssuesOptions{ Labels: "1,2", Page: 1, PageSize: 4, }, - []int64{5, 2, 1}, + []int64{1}, }, } { issues, err := Issues(&test.Opts) From 35e0f5b3cc02907c414157bf7c87e86563d75641 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 22 May 2018 20:11:03 +0800 Subject: [PATCH 6/7] Update issue_test.go to describe why the result is {1} instead of {5, 2, 1} --- models/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_test.go b/models/issue_test.go index b0855c4504f48..a47baeacde131 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -205,7 +205,7 @@ func TestIssues(t *testing.T) { Page: 1, PageSize: 4, }, - []int64{1}, + []int64{1}, // issues with **both** label 1 and 2, only issue 1 matches }, } { issues, err := Issues(&test.Opts) From 5f08f1bc0ac685c8e0fd983ca652ee363625395f Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 22 May 2018 21:13:10 +0800 Subject: [PATCH 7/7] fix test --- models/fixtures/issue_label.yml | 5 ----- models/issue_test.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/models/fixtures/issue_label.yml b/models/fixtures/issue_label.yml index 1f0a91ad998b5..49d5a95d020f3 100644 --- a/models/fixtures/issue_label.yml +++ b/models/fixtures/issue_label.yml @@ -12,8 +12,3 @@ id: 3 issue_id: 2 label_id: 1 - -- - id: 4 - issue_id: 1 - label_id: 2 diff --git a/models/issue_test.go b/models/issue_test.go index a47baeacde131..c30d4cbd129c7 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -205,7 +205,7 @@ func TestIssues(t *testing.T) { Page: 1, PageSize: 4, }, - []int64{1}, // issues with **both** label 1 and 2, only issue 1 matches + []int64{}, // issues with **both** label 1 and 2, none of these issues matches, TODO: add more tests }, } { issues, err := Issues(&test.Opts)