Skip to content

Commit dd45362

Browse files
brechtvlbrechtvl
committed
Scoped labels
Out of any labels named like scope::name, ensure at most one can be applied to an issue. The scope is determined by the last occurence of ::, so for example scope::alpha::name and scope::beta::name are considered to be in different scopes and can coexist. The :: syntax matches the same scoped labels feature in GitLab. Exclusive scopes are not enforced by any database rules, however they are enforced when editing labels at the models level, automatically removing any existing labels in the same scope when either attaching a new label or replacing all labels. In menus, scoped labels are grouped with a divider line and use a circle instead of checkbox to indicate they function as radio buttons per scope. Issue filtering by label ensures that only a single scoped label is selected at a time. Clicking with alt key can be used to remove a scoped label, both when editing individual issues and batch editing. This functionality can break existing setups if labels included :: but were not intended as exclusive.
1 parent 29b78bc commit dd45362

File tree

18 files changed

+323
-64
lines changed

18 files changed

+323
-64
lines changed

‎models/fixtures/issue.yml

+17
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,20 @@
287287
created_unix: 1602935696
288288
updated_unix: 1602935696
289289
is_locked: false
290+
291+
-
292+
id: 18
293+
repo_id: 55
294+
index: 1
295+
poster_id: 2
296+
original_author_id: 0
297+
name: issue for scoped labels
298+
content: content
299+
milestone_id: 0
300+
priority: 0
301+
is_closed: false
302+
is_pull: false
303+
num_comments: 0
304+
created_unix: 946684830
305+
updated_unix: 978307200
306+
is_locked: false

‎models/fixtures/label.yml

+36
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,39 @@
4242
color: '#000000'
4343
num_issues: 0
4444
num_closed_issues: 0
45+
46+
-
47+
id: 6
48+
repo_id: 55
49+
org_id: 0
50+
name: unscoped_label
51+
color: '#000000'
52+
num_issues: 0
53+
num_closed_issues: 0
54+
55+
-
56+
id: 7
57+
repo_id: 55
58+
org_id: 0
59+
name: scope::label1
60+
color: '#000000'
61+
num_issues: 0
62+
num_closed_issues: 0
63+
64+
-
65+
id: 8
66+
repo_id: 55
67+
org_id: 0
68+
name: scope::label2
69+
color: '#000000'
70+
num_issues: 0
71+
num_closed_issues: 0
72+
73+
-
74+
id: 9
75+
repo_id: 55
76+
org_id: 0
77+
name: scope::subscope::label2
78+
color: '#000000'
79+
num_issues: 0
80+
num_closed_issues: 0

‎models/fixtures/repository.yml

+12
Original file line numberDiff line numberDiff line change
@@ -1596,3 +1596,15 @@
15961596
is_archived: false
15971597
is_private: true
15981598
status: 0
1599+
1600+
-
1601+
id: 55
1602+
owner_id: 2
1603+
owner_name: user2
1604+
lower_name: scoped_label
1605+
name: scoped_label
1606+
is_empty: false
1607+
is_archived: false
1608+
is_private: true
1609+
num_issues: 1
1610+
status: 0

‎models/fixtures/user.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
num_followers: 2
6767
num_following: 1
6868
num_stars: 2
69-
num_repos: 10
69+
num_repos: 11
7070
num_teams: 0
7171
num_members: 0
7272
visibility: 0

‎models/issues/issue.go

+27
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,31 @@ func (ts labelSorter) Swap(i, j int) {
538538
[]*Label(ts)[i], []*Label(ts)[j] = []*Label(ts)[j], []*Label(ts)[i]
539539
}
540540

541+
// Ensure only one label of a given scope exists, with labels at the end of the
542+
// array getting preference over earlier ones.
543+
func RemoveDuplicateScopeLabels(labels []*Label) []*Label {
544+
var validLabels []*Label
545+
546+
for i, label := range labels {
547+
scope := label.Scope()
548+
if scope != "" {
549+
foundOther := false
550+
for _, otherLabel := range labels[i+1:] {
551+
if otherLabel.Scope() == scope {
552+
foundOther = true
553+
break
554+
}
555+
}
556+
if foundOther {
557+
continue
558+
}
559+
}
560+
validLabels = append(validLabels, label)
561+
}
562+
563+
return validLabels
564+
}
565+
541566
// ReplaceIssueLabels removes all current labels and add new labels to the issue.
542567
// Triggers appropriate WebHooks, if any.
543568
func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (err error) {
@@ -555,6 +580,8 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
555580
return err
556581
}
557582

583+
labels = RemoveDuplicateScopeLabels(labels)
584+
558585
sort.Sort(labelSorter(labels))
559586
sort.Sort(labelSorter(issue.Labels))
560587

‎models/issues/issue_test.go

+12-7
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
func TestIssue_ReplaceLabels(t *testing.T) {
2626
assert.NoError(t, unittest.PrepareTestDatabase())
2727

28-
testSuccess := func(issueID int64, labelIDs []int64) {
28+
testSuccess := func(issueID int64, labelIDs, expectedLabelIDs []int64) {
2929
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueID})
3030
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
3131
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
@@ -35,15 +35,20 @@ func TestIssue_ReplaceLabels(t *testing.T) {
3535
labels[i] = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: labelID, RepoID: repo.ID})
3636
}
3737
assert.NoError(t, issues_model.ReplaceIssueLabels(issue, labels, doer))
38-
unittest.AssertCount(t, &issues_model.IssueLabel{IssueID: issueID}, len(labelIDs))
39-
for _, labelID := range labelIDs {
38+
unittest.AssertCount(t, &issues_model.IssueLabel{IssueID: issueID}, len(expectedLabelIDs))
39+
for _, labelID := range expectedLabelIDs {
4040
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID})
4141
}
4242
}
4343

44-
testSuccess(1, []int64{2})
45-
testSuccess(1, []int64{1, 2})
46-
testSuccess(1, []int64{})
44+
testSuccess(1, []int64{2}, []int64{2})
45+
testSuccess(1, []int64{1, 2}, []int64{1, 2})
46+
testSuccess(1, []int64{}, []int64{})
47+
48+
// mutually exclusive scoped labels 7 and 8
49+
testSuccess(18, []int64{6, 7}, []int64{6, 7})
50+
testSuccess(18, []int64{7, 8}, []int64{8})
51+
testSuccess(18, []int64{6, 8, 7}, []int64{6, 7})
4752
}
4853

4954
func Test_GetIssueIDsByRepoID(t *testing.T) {
@@ -523,5 +528,5 @@ func TestCountIssues(t *testing.T) {
523528
assert.NoError(t, unittest.PrepareTestDatabase())
524529
count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{})
525530
assert.NoError(t, err)
526-
assert.EqualValues(t, 17, count)
531+
assert.EqualValues(t, 18, count)
527532
}

‎models/issues/label.go

+44-3
Original file line numberDiff line numberDiff line change
@@ -128,18 +128,22 @@ func (label *Label) CalOpenOrgIssues(ctx context.Context, repoID, labelID int64)
128128
}
129129

130130
// LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked
131-
func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) {
131+
func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, currentSelectedScopes []string) {
132132
var labelQuerySlice []string
133133
labelSelected := false
134134
labelID := strconv.FormatInt(label.ID, 10)
135-
for _, s := range currentSelectedLabels {
135+
labelScope := label.Scope()
136+
for i, s := range currentSelectedLabels {
136137
if s == label.ID {
137138
labelSelected = true
138139
} else if -s == label.ID {
139140
labelSelected = true
140141
label.IsExcluded = true
141142
} else if s != 0 {
142-
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10))
143+
// Exclude other labels in the same scope from selection
144+
if s < 0 || labelScope == "" || labelScope != currentSelectedScopes[i] {
145+
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10))
146+
}
143147
}
144148
}
145149
if !labelSelected {
@@ -204,6 +208,16 @@ func (label *Label) ForegroundColor() template.CSS {
204208
return template.CSS("#000")
205209
}
206210

211+
// Return scope substring of label name, or empty string if none exists.
212+
func (label *Label) Scope() string {
213+
lastIndex := strings.LastIndex(label.Name, "::")
214+
if lastIndex == -1 {
215+
return ""
216+
}
217+
218+
return label.Name[:lastIndex+2]
219+
}
220+
207221
// NewLabel creates a new label
208222
func NewLabel(ctx context.Context, label *Label) error {
209223
if !LabelColorPattern.MatchString(label.Color) {
@@ -620,6 +634,29 @@ func newIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
620634
return updateLabelCols(ctx, label, "num_issues", "num_closed_issue")
621635
}
622636

637+
// Remove all issue labels in the given scope
638+
func RemoveSameScopeIssueLabels(ctx context.Context, issue *Issue, label *Label, doer *user_model.User) (err error) {
639+
scope := label.Scope()
640+
if scope == "" {
641+
return nil
642+
}
643+
644+
var toRemove []*Label
645+
for _, issueLabel := range issue.Labels {
646+
if label.ID != issueLabel.ID && issueLabel.Scope() == scope {
647+
toRemove = append(toRemove, issueLabel)
648+
}
649+
}
650+
651+
for _, issueLabel := range toRemove {
652+
if err = deleteIssueLabel(ctx, issue, issueLabel, doer); err != nil {
653+
return err
654+
}
655+
}
656+
657+
return nil
658+
}
659+
623660
// NewIssueLabel creates a new issue-label relation.
624661
func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error) {
625662
if HasIssueLabel(db.DefaultContext, issue.ID, label.ID) {
@@ -641,6 +678,10 @@ func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error
641678
return nil
642679
}
643680

681+
if err = RemoveSameScopeIssueLabels(ctx, issue, label, doer); err != nil {
682+
return nil
683+
}
684+
644685
if err = newIssueLabel(ctx, issue, label, doer); err != nil {
645686
return err
646687
}

‎models/issues/label_test.go

+37
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ func TestLabel_ForegroundColor(t *testing.T) {
3434
assert.Equal(t, template.CSS("#fff"), label.ForegroundColor())
3535
}
3636

37+
func TestLabel_Scope(t *testing.T) {
38+
assert.NoError(t, unittest.PrepareTestDatabase())
39+
label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 7})
40+
assert.Equal(t, "scope::", label.Scope())
41+
42+
label = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 9})
43+
assert.Equal(t, "scope::subscope::", label.Scope())
44+
}
45+
3746
func TestNewLabels(t *testing.T) {
3847
assert.NoError(t, unittest.PrepareTestDatabase())
3948
labels := []*issues_model.Label{
@@ -323,6 +332,34 @@ func TestNewIssueLabel(t *testing.T) {
323332
unittest.CheckConsistencyFor(t, &issues_model.Issue{}, &issues_model.Label{})
324333
}
325334

335+
func TestNewIssueLabelScoped(t *testing.T) {
336+
assert.NoError(t, unittest.PrepareTestDatabase())
337+
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 18})
338+
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
339+
340+
unscopedLabel := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 6})
341+
scopedLabelA := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 7})
342+
scopedLabelB := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 8})
343+
344+
// coexisting unscoped and scoped label
345+
assert.NoError(t, issues_model.NewIssueLabel(issue, unscopedLabel, doer))
346+
assert.NoError(t, issues_model.NewIssueLabel(issue, scopedLabelA, doer))
347+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: unscopedLabel.ID})
348+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: scopedLabelA.ID})
349+
350+
// scoped label replaces existing one
351+
assert.NoError(t, issues_model.NewIssueLabel(issue, scopedLabelB, doer))
352+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: unscopedLabel.ID})
353+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: scopedLabelB.ID})
354+
unittest.AssertNotExistsBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: scopedLabelA.ID})
355+
356+
// scoped label replaces existing one again
357+
assert.NoError(t, issues_model.NewIssueLabel(issue, scopedLabelA, doer))
358+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: unscopedLabel.ID})
359+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: scopedLabelA.ID})
360+
unittest.AssertNotExistsBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: scopedLabelB.ID})
361+
}
362+
326363
func TestNewIssueLabels(t *testing.T) {
327364
assert.NoError(t, unittest.PrepareTestDatabase())
328365
label1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})

‎routers/web/repo/issue.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,23 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
331331
labels = append(labels, orgLabels...)
332332
}
333333

334+
var labelScopes []string
335+
for _, labelID := range labelIDs {
336+
foundScope := false
337+
for _, label := range labels {
338+
if label.ID == labelID || label.ID == -labelID {
339+
labelScopes = append(labelScopes, label.Scope())
340+
foundScope = true
341+
break
342+
}
343+
}
344+
if !foundScope {
345+
labelScopes = append(labelScopes, "")
346+
}
347+
}
348+
334349
for _, l := range labels {
335-
l.LoadSelectedLabelsAfterClick(labelIDs)
350+
l.LoadSelectedLabelsAfterClick(labelIDs, labelScopes)
336351
}
337352
ctx.Data["Labels"] = labels
338353
ctx.Data["NumLabels"] = len(labels)

‎routers/web/repo/issue_label.go

+11-5
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func UpdateIssueLabel(ctx *context.Context) {
175175
return
176176
}
177177
}
178-
case "attach", "detach", "toggle":
178+
case "attach", "detach", "toggle", "toggle-alt":
179179
label, err := issues_model.GetLabelByID(ctx, ctx.FormInt64("id"))
180180
if err != nil {
181181
if issues_model.IsErrRepoLabelNotExist(err) {
@@ -189,12 +189,18 @@ func UpdateIssueLabel(ctx *context.Context) {
189189
if action == "toggle" {
190190
// detach if any issues already have label, otherwise attach
191191
action = "attach"
192-
for _, issue := range issues {
193-
if issues_model.HasIssueLabel(ctx, issue.ID, label.ID) {
194-
action = "detach"
195-
break
192+
if label.Scope() == "" {
193+
for _, issue := range issues {
194+
if issues_model.HasIssueLabel(ctx, issue.ID, label.ID) {
195+
action = "detach"
196+
break
197+
}
196198
}
197199
}
200+
} else if action == "toggle-alt" {
201+
// always detach with alt key pressed, to be able to remove
202+
// scoped labels
203+
action = "detach"
198204
}
199205

200206
if action == "attach" {

‎templates/repo/issue/list.tmpl

+14-2
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,14 @@
5151
</div>
5252
<span class="info">{{.locale.Tr "repo.issues.filter_label_exclude" | Safe}}</span>
5353
<a class="item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}">{{.locale.Tr "repo.issues.filter_label_no_select"}}</a>
54+
{{$previousScope := "_no_scope"}}
5455
{{range .Labels}}
55-
<a class="item label-filter-item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}" data-label-id="{{.ID}}">{{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if .IsSelected}}{{svg "octicon-check"}}{{end}}<span class="label color" style="background-color: {{.Color}}"></span> {{.Name | RenderEmoji}}</a>
56+
{{$scope := .Scope}}
57+
{{if and (ne $previousScope "_no_scope") (ne $previousScope $scope)}}
58+
<div class="ui divider"></div>
59+
{{end}}
60+
{{$previousScope = $scope}}
61+
<a class="item label-filter-item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}" data-label-id="{{.ID}}">{{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if .IsSelected}}{{if $scope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}}<span class="label color" style="background-color: {{.Color}}"></span> {{.Name | RenderEmoji}}</a>
5662
{{end}}
5763
</div>
5864
</div>
@@ -178,9 +184,15 @@
178184
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
179185
</span>
180186
<div class="menu">
187+
{{$previousScope := "_no_scope"}}
181188
{{range .Labels}}
189+
{{$scope := .Scope}}
190+
{{if and (ne $previousScope "_no_scope") (ne $previousScope $scope)}}
191+
<div class="ui divider"></div>
192+
{{end}}
193+
{{$previousScope = $scope}}
182194
<div class="item issue-action" data-action="toggle" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/labels">
183-
{{if contain $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}}<span class="label color" style="background-color: {{.Color}}"></span> {{.Name | RenderEmoji}}
195+
{{if contain $.SelLabelIDs .ID}}{{if $scope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}}<span class="label color" style="background-color: {{.Color}}"></span> {{.Name | RenderEmoji}}
184196
</div>
185197
{{end}}
186198
</div>

0 commit comments

Comments
 (0)