From a814cf756755a018df4353783bb89c345e54f447 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 22 Oct 2026 14:40:40 -0700 Subject: [PATCH] fix(api): adding issue labels by name attaches both the repo and the org label (#39873) (#39951) Backport #39873 by @nazem-droubi Fixes #39651 When an org-owned repo has a repo label and an org label with the same name, `POST`/`PUT /repos/{owner}/{repo}/issues/{index}/labels` with label names attached both of them. With this change the repo label is used when the names collide, and org labels are only looked up for the names that didn't match a repo label. Names that only exist at the org level still resolve to the org label as before. I documented the behavior on `IssueLabelsOption.Labels` and regenerated the swagger files. Picking the repo label seemed the least surprising choice since it's the more specific one, but I'm happy to change it (for example to return a 422 on ambiguous names) if you'd prefer that. Tests: added `TestAPIAddIssueLabelsWithSharedRepoAndOrgLabelName`, which covers both POST and PUT. It fails on `main` (3 labels attached instead of 2) and passes with this change. I ran the existing add/replace label integration tests locally on sqlite, plus `go vet` and gofmt. AI disclosure: the code and test in this PR were written primarily by an AI coding agent working on my behalf. I'm happy to walk through it or revise anything during review. Signed-off-by: Nazem <214302554+nazem-droubi@users.noreply.github.com> Co-authored-by: Nazem Co-authored-by: Nazem <214302554+nazem-droubi@users.noreply.github.com> Co-authored-by: silverwind --- modules/structs/issue_label.go | 4 ++- routers/api/v1/repo/issue_label.go | 16 +++++++-- templates/swagger/v1-openapi3.generated.json | 2 +- templates/swagger/v1-swagger.generated.json | 2 +- tests/integration/api_issue_label_test.go | 34 ++++++++++++++++++++ 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/modules/structs/issue_label.go b/modules/structs/issue_label.go index 16bd0b3c94a..00a17e01baa 100644 --- a/modules/structs/issue_label.go +++ b/modules/structs/issue_label.go @@ -56,7 +56,9 @@ type EditLabelOption struct { // IssueLabelsOption a collection of labels type IssueLabelsOption struct { // Labels can be a list of integers representing label IDs - // or a list of strings representing label names + // or a list of strings representing label names. + // If a repository label and an organization label share a name, + // the repository label is used. Labels []any `json:"labels"` } diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 88632d9eabb..a1f285a91b0 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -10,6 +10,7 @@ import ( "reflect" issues_model "gitea.dev/models/issues" + "gitea.dev/modules/container" api "gitea.dev/modules/structs" "gitea.dev/modules/web" "gitea.dev/services/context" @@ -339,12 +340,23 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) } labelIDs = append(labelIDs, repoLabelIDs...) if ctx.Repo.Owner.IsOrganization() { - orgLabelIDs, err := issues_model.GetLabelIDsInOrgByNames(ctx, ctx.Repo.Owner.ID, labelNames) + repoLabels, err := issues_model.GetLabelsByIDs(ctx, repoLabelIDs, "name") if err != nil { ctx.APIErrorInternal(err) return nil, nil, err } - labelIDs = append(labelIDs, orgLabelIDs...) + orgLabelNames := container.SetOf(labelNames...) + for _, label := range repoLabels { + orgLabelNames.Remove(label.Name) + } + if len(orgLabelNames) > 0 { + orgLabelIDs, err := issues_model.GetLabelIDsInOrgByNames(ctx, ctx.Repo.Owner.ID, orgLabelNames.Values()) + if err != nil { + ctx.APIErrorInternal(err) + return nil, nil, err + } + labelIDs = append(labelIDs, orgLabelIDs...) + } } } diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index ecfe5f8f377..787e2569aea 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -7416,7 +7416,7 @@ "description": "IssueLabelsOption a collection of labels", "properties": { "labels": { - "description": "Labels can be a list of integers representing label IDs\nor a list of strings representing label names", + "description": "Labels can be a list of integers representing label IDs\nor a list of strings representing label names.\nIf a repository label and an organization label share a name,\nthe repository label is used.", "items": {}, "type": "array", "x-go-name": "Labels" diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index 98f81a99c00..03d590c4077 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -30525,7 +30525,7 @@ "type": "object", "properties": { "labels": { - "description": "Labels can be a list of integers representing label IDs\nor a list of strings representing label names", + "description": "Labels can be a list of integers representing label IDs\nor a list of strings representing label names.\nIf a repository label and an organization label share a name,\nthe repository label is used.", "type": "array", "items": {}, "x-go-name": "Labels" diff --git a/tests/integration/api_issue_label_test.go b/tests/integration/api_issue_label_test.go index 1f9d0086ae9..35a2873ce78 100644 --- a/tests/integration/api_issue_label_test.go +++ b/tests/integration/api_issue_label_test.go @@ -175,6 +175,40 @@ func TestAPIAddIssueLabelsWithLabelNames(t *testing.T) { MakeRequest(t, req, http.StatusNoContent) } +func TestAPIAddIssueLabelsWithSharedRepoAndOrgLabelName(t *testing.T) { + assert.NoError(t, unittest.LoadFixtures()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 6, RepoID: repo.ID}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + repoLabel := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 10, RepoID: repo.ID}) + orgOnlyLabel := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 4, OrgID: owner.ID}) + + assert.NoError(t, issues_model.NewLabel(t.Context(), &issues_model.Label{OrgID: owner.ID, Name: repoLabel.Name, Color: "#abcdef"})) + + token := getTokenForLoggedInUser(t, loginUser(t, "user1"), auth_model.AccessTokenScopeWriteIssue) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels", owner.Name, repo.Name, issue.Index) + + for _, method := range []string{"POST", "PUT"} { + t.Run(method, func(t *testing.T) { + req := NewRequest(t, "DELETE", urlStr).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + req = NewRequestWithJSON(t, method, urlStr, &api.IssueLabelsOption{ + Labels: []any{repoLabel.Name, orgOnlyLabel.Name}, + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + apiLabels := DecodeJSON(t, resp, []*api.Label{}) + + var apiLabelIDs []int64 + for _, label := range apiLabels { + apiLabelIDs = append(apiLabelIDs, label.ID) + } + assert.ElementsMatch(t, []int64{repoLabel.ID, orgOnlyLabel.ID}, apiLabelIDs) + }) + } +} + func TestAPIReplaceIssueLabels(t *testing.T) { assert.NoError(t, unittest.LoadFixtures())