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

Add the ability to use multiple labels as filters #3438

Closed
wants to merge 7 commits into from

Conversation

erjiaqing
Copy link

Implements Issue #3430

multiplelabels

As in GitHub, when selecting multiple labels, only display those issues with all the labels selected.

I cannot figure out how to achieve this using the orm provided, so I write a subquery.

Then I use Label.QueryString and Label.IsSelected for the query string(issues?labels=xxx) and the tick before labels.

@lunny lunny added the type/enhancement An improvement of existing functionality label Jan 31, 2018
@lunny lunny added this to the 1.x.x milestone Jan 31, 2018
@@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should rename query []string to labels []string.
I think that the function name is not very self-explanatory at the first place. It should be renamed into RestoreSelectedLabels or LoadSelectedLabelsFromQuery.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 2, 2018
@erjiaqing
Copy link
Author

In GitHub, when multiple labels are selected, only issues with all of the selected labels will be shown. However, in test, it is assumed that issues with at least one of the selected labels will be shown.
Maybe I have to change the tests?

@erjiaqing erjiaqing closed this Feb 3, 2018
@erjiaqing erjiaqing reopened this Feb 3, 2018
when multiple labels selected, only issues with all the selected labels will be shown
@ernierasta
Copy link

Drone will not show, what is the problem with CI... so I have no idea how serious problem with this commit is ...

This would be life saver, is there reason why it is not merged?
Can we make it merge? ;-)

to describe why the result is {1} instead of {5, 2, 1}
@erjiaqing
Copy link
Author

I could remember that the last time, it reports that my commit cannot pass the unit test
it is because that:

 			IssuesOptions{
 				Labels:   "1,2",
 				Page:     1,
 				PageSize: 4,
 			},
-			[]int64{5, 2, 1},
+			[]int64{1}, // issues with **both** label 1 and 2, only issue 1 matches
 		},

I made a net commit, and it should pass the test now, and this time, it reports another failure
I have no idea about this, maybe the structure of database had changed a lot
@ernierasta
(I don't know if you will be notified for this)

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@45f3d4a). Click here to learn what that means.
The diff coverage is 13.63%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3438   +/-   ##
=========================================
  Coverage          ?   20.11%           
=========================================
  Files             ?      153           
  Lines             ?    30393           
  Branches          ?        0           
=========================================
  Hits              ?     6114           
  Misses            ?    23372           
  Partials          ?      907
Impacted Files Coverage Δ
routers/repo/issue_label.go 42.76% <0%> (ø)
models/issue.go 32.06% <11.76%> (ø)
models/issue_label.go 56.1% <30.76%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45f3d4a...5f08f1b. Read the comment docs.

} else if len(labelIDs) > 1 {
cond, args, _ := builder.ToSQL(builder.In("issue_label.label_id", labelIDs))
sess.
Where(fmt.Sprintf(`issue.id IN (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be built using builder? cc: @lunny

@yasuokav
Copy link
Contributor

https://github.com/erjiaqing/gitea/blob/5f08f1bc0ac685c8e0fd983ca652ee363625395f/templates/repo/issue/list.tmpl#L135

<span class="octicon {{if eq $.SelectLabels .ID}}octicon-check{{end}}"></span><span class="label color" style="background-color: {{.Color}}"></span> {{.Name}}

eq $.SelectLabels .ID -> .IsSelected?

https://github.com/erjiaqing/gitea/blob/5f08f1bc0ac685c8e0fd983ca652ee363625395f/routers/repo/issue.go#L243

ctx.Data["SelectLabels"] = com.StrTo(selectLabels).MustInt64()

selectLabels is no longer an int

@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query could be fixed to do > 0 instead of having 2 different queries

@stale
Copy link

stale bot commented Jan 5, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 5, 2019
@stale
Copy link

stale bot commented Jan 19, 2019

This pull request has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this Jan 19, 2019
@lafriks lafriks removed this from the 1.x.x milestone Jan 21, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants