-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Conversation
models/issue_label.go
Outdated
@@ -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) { |
There was a problem hiding this comment.
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
.
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. |
when multiple labels selected, only issues with all the selected labels will be shown
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? |
to describe why the result is {1} instead of {5, 2, 1}
I could remember that the last time, it reports that my commit cannot pass the unit test
I made a net commit, and it should pass the test now, and this time, it reports another failure |
Codecov Report
@@ Coverage Diff @@
## master #3438 +/- ##
=========================================
Coverage ? 20.11%
=========================================
Files ? 153
Lines ? 30393
Branches ? 0
=========================================
Hits ? 6114
Misses ? 23372
Partials ? 907
Continue to review full report at Codecov.
|
} else if len(labelIDs) > 1 { | ||
cond, args, _ := builder.ToSQL(builder.In("issue_label.label_id", labelIDs)) | ||
sess. | ||
Where(fmt.Sprintf(`issue.id IN ( |
There was a problem hiding this comment.
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
|
@@ -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 { |
There was a problem hiding this comment.
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
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. |
This pull request has been automatically closed because of inactivity. You can re-open it if needed. |
Implements Issue #3430
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
andLabel.IsSelected
for the query string(issues?labels=xxx) and the tick before labels.