mirror of
https://gitea.com/gitea/tea.git
synced 2026-10-26 19:11:25 +01:00
fix(label): refuse duplicate label names unless --force
tea label create --name bug twice created a second "bug" label, after which issue edit --add-labels bug failed as ambiguous and issue list --label bug matched nothing. label create (and --file imports) now fail with "label "bug" already exists in alice/project; pass --force to update it", ignoring case; --force updates the existing label. The ambiguity error for labels that already share a name lists their IDs. Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
5 files changed
+92
-10
No files matched your search
@@ -15,6 +15,10 @@ import (
|
||||
func TestLabelCreateAndEditExclusiveArchived(t *testing.T) {
|
||||
calls := 0
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodGet {
|
||||
_, _ = w.Write([]byte(`[]`))
|
||||
return
|
||||
}
|
||||
calls++
|
||||
var payload map[string]any
|
||||
require.NoError(t, json.NewDecoder(r.Body).Decode(&payload))
|
||||
@@ -47,3 +51,35 @@ func TestLabelCreateAndEditExclusiveArchived(t *testing.T) {
|
||||
require.Equal(t, 2, calls)
|
||||
require.Empty(t, diagnostics.String())
|
||||
}
|
||||
|
||||
func TestLabelCreateRefusesDuplicateNames(t *testing.T) {
|
||||
var changed []string
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.Method + " " + r.URL.Path {
|
||||
case "GET /api/v1/repos/alice/project/labels":
|
||||
if r.URL.Query().Get("page") == "1" {
|
||||
_, _ = w.Write([]byte(`[{"id":11,"name":"bug","color":"ee0701","description":"Broken"}]`))
|
||||
return
|
||||
}
|
||||
_, _ = w.Write([]byte(`[]`))
|
||||
case "PATCH /api/v1/repos/alice/project/labels/11":
|
||||
requirePayload(t, `{"color":"000000"}`, r.Body)
|
||||
changed = append(changed, r.Method+" "+r.URL.Path)
|
||||
_, _ = w.Write([]byte(`{"id":11,"name":"bug","color":"000000","description":"Broken"}`))
|
||||
default:
|
||||
changed = append(changed, r.Method+" "+r.URL.Path)
|
||||
_, _ = w.Write([]byte(`{"id":12,"name":"docs"}`))
|
||||
}
|
||||
}))
|
||||
defer server.Close()
|
||||
deps, out, _ := serverDeps(t, server.URL)
|
||||
for _, name := range []string{"bug", "BUG"} {
|
||||
err := Execute(context.Background(), deps, []string{"label", "create", "-R", "alice/project", "--name", name, "--color", "000000"})
|
||||
require.EqualError(t, err, `label "bug" already exists in alice/project; pass --force to update it`)
|
||||
}
|
||||
require.Empty(t, changed)
|
||||
require.Empty(t, out.String())
|
||||
require.NoError(t, Execute(context.Background(), deps, []string{"label", "create", "-R", "alice/project", "--name", "bug", "--color", "000000", "--force", "--json=id,color"}))
|
||||
require.JSONEq(t, `{"id":11,"color":"000000"}`, out.String())
|
||||
require.Equal(t, []string{"PATCH /api/v1/repos/alice/project/labels/11"}, changed)
|
||||
}
|
||||
@@ -51,6 +51,10 @@ func TestLabelImportAcceptsColorPrefixedRows(t *testing.T) {
|
||||
var created []string
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
require.Equal(t, "/api/v1/repos/alice/project/labels", r.URL.Path)
|
||||
if r.Method == http.MethodGet {
|
||||
_, _ = w.Write([]byte(`[]`))
|
||||
return
|
||||
}
|
||||
payload, err := io.ReadAll(r.Body)
|
||||
require.NoError(t, err)
|
||||
var label struct{ Name string }
|
||||
|
||||
@@ -45,7 +45,7 @@ func (a *application) labelCommand() *cobra.Command {
|
||||
group.AddCommand(list)
|
||||
for _, verb := range []string{"create", "edit"} {
|
||||
var name, color, description, file string
|
||||
var exclusive, archived bool
|
||||
var exclusive, archived, force bool
|
||||
command := &cobra.Command{Use: verb, Short: titleCase(verb) + " a label"}
|
||||
if verb == "edit" {
|
||||
command.Use += " <id>"
|
||||
@@ -60,6 +60,7 @@ func (a *application) labelCommand() *cobra.Command {
|
||||
command.Flags().BoolVar(&archived, "archived", false, "Archive the label so it cannot be newly applied")
|
||||
if verb == "create" {
|
||||
command.Flags().StringVar(&file, "file", "", "Import color/name rows with optional ;description")
|
||||
command.Flags().BoolVar(&force, "force", false, "Update the label if one with the same name exists")
|
||||
}
|
||||
command.RunE = func(cmd *cobra.Command, args []string) error {
|
||||
opts := services.LabelOptions{}
|
||||
@@ -106,10 +107,10 @@ func (a *application) labelCommand() *cobra.Command {
|
||||
}
|
||||
opts.Color = &color
|
||||
}
|
||||
if verb == "create" || cmd.Flags().Changed("description") {
|
||||
if cmd.Flags().Changed("description") {
|
||||
opts.Description = &description
|
||||
}
|
||||
if verb == "create" || cmd.Flags().Changed("exclusive") {
|
||||
if cmd.Flags().Changed("exclusive") {
|
||||
opts.Exclusive = &exclusive
|
||||
}
|
||||
if cmd.Flags().Changed("archived") {
|
||||
@@ -123,22 +124,28 @@ func (a *application) labelCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var existing []services.Label
|
||||
if verb == "create" {
|
||||
if existing, err = s.All(cmd.Context()); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if file != "" {
|
||||
results := []services.Label{}
|
||||
var failures []error
|
||||
for _, row := range imported {
|
||||
item, err := s.Create(cmd.Context(), row)
|
||||
item, err := s.CreateUnique(cmd.Context(), existing, row, force)
|
||||
if err != nil {
|
||||
failures = append(failures, fmt.Errorf("label %s: %w", *row.Name, err))
|
||||
continue
|
||||
}
|
||||
results = append(results, item)
|
||||
results, existing = append(results, item), append(existing, item)
|
||||
}
|
||||
return errors.Join(append(failures, writeList(a, cmd, results, labelRow))...)
|
||||
}
|
||||
var item services.Label
|
||||
if verb == "create" {
|
||||
item, err = s.Create(cmd.Context(), opts)
|
||||
item, err = s.CreateUnique(cmd.Context(), existing, opts, force)
|
||||
} else {
|
||||
item, err = s.Edit(cmd.Context(), id, opts)
|
||||
}
|
||||
|
||||
@@ -5,11 +5,46 @@ package services
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
func selectLabelNames(names []string, matches map[string][]int64) ([]int64, error) {
|
||||
// Scope names the repository or organization the labels belong to.
|
||||
func (s Labels) Scope() string {
|
||||
if s.Organization != "" {
|
||||
return s.Organization
|
||||
}
|
||||
return s.Repository
|
||||
}
|
||||
|
||||
// finder addresses the labels of the scope by ID or name (D10); byName matches numeric values as
|
||||
// names only.
|
||||
func (s Labels) finder(byName bool) NameOrID[Label] {
|
||||
return NameOrID[Label]{Kind: "label", Scope: s.Scope(), ID: func(l Label) int64 { return l.ID }, Name: func(l Label) string { return l.Name }, ByName: byName}
|
||||
}
|
||||
|
||||
// CreateUnique creates a label unless existing, the labels of the scope, has one with its name,
|
||||
// ignoring case. Then update edits that label with the supplied fields, keeping its name, and
|
||||
// otherwise CreateUnique fails: Gitea would create a second label that name filters cannot tell
|
||||
// apart.
|
||||
func (s Labels) CreateUnique(ctx context.Context, existing []Label, opts LabelOptions, update bool) (Label, error) {
|
||||
current, err := s.finder(true).Find(existing, deref(opts.Name))
|
||||
if errors.As(err, new(*NotFoundError)) {
|
||||
return s.Create(ctx, opts)
|
||||
}
|
||||
if err != nil {
|
||||
return Label{}, err
|
||||
}
|
||||
if !update {
|
||||
return Label{}, &ConflictError{Reason: ReasonExists, Kind: "label", Key: strconv.Quote(current.Name), Scope: s.Scope(), Hint: "pass --force to update it"}
|
||||
}
|
||||
opts.Name = nil
|
||||
return s.Edit(ctx, current.ID, opts)
|
||||
}
|
||||
|
||||
func selectLabelNames(scope string, names []string, matches map[string][]int64) ([]int64, error) {
|
||||
ids := make([]int64, 0, len(names))
|
||||
seen := map[int64]bool{}
|
||||
for _, name := range names {
|
||||
@@ -18,7 +53,7 @@ func selectLabelNames(names []string, matches map[string][]int64) ([]int64, erro
|
||||
return nil, fmt.Errorf("label %q does not exist; use tea label list", name)
|
||||
}
|
||||
if len(candidates) > 1 {
|
||||
return nil, fmt.Errorf("label %q is ambiguous", name)
|
||||
return nil, &AmbiguousError{Kind: "label", Key: strconv.Quote(name), Scope: scope, IDs: candidates}
|
||||
}
|
||||
if !seen[candidates[0]] {
|
||||
ids = append(ids, candidates[0])
|
||||
@@ -70,7 +105,7 @@ func (s Labels) ResolveIssueNames(ctx context.Context, names []string) ([]int64,
|
||||
scope, name := splitLabelScope(input)
|
||||
matches[input] = index[labelKey{scope, name}]
|
||||
}
|
||||
return selectLabelNames(names, matches)
|
||||
return selectLabelNames(s.Repository, names, matches)
|
||||
}
|
||||
|
||||
func splitLabelScope(input string) (scope, name string) {
|
||||
|
||||
@@ -66,7 +66,7 @@ func TestIssueLabelsCombineScopesWithoutSilentlyChoosingDuplicates(t *testing.T)
|
||||
}))
|
||||
service := Labels{Client: client, Repository: "company/project"}
|
||||
_, err := service.ResolveIssueNames(context.Background(), []string{"bug"})
|
||||
require.ErrorContains(t, err, "ambiguous")
|
||||
require.EqualError(t, err, `label "bug" is ambiguous in company/project (IDs 1, 2); pass an ID`)
|
||||
ids, err := service.ResolveIssueNames(context.Background(), []string{"org:bug", "repo:bug", "shared", "shared"})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []int64{2, 1, 3}, ids)
|
||||
|
||||
Reference in new issue
Block a user