fix(api): adding issue labels by name attaches both the repo and the org label (#39873)

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.

---------

Signed-off-by: Nazem <214302554+nazem-droubi@users.noreply.github.com>
Co-authored-by: Nazem <214302554+nazem-droubi@users.noreply.github.com>
Co-authored-by: silverwind <me@silverwind.io>

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"`
 }
 

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...)
+			}
 		}
 	}
 

templates/swagger/v1-openapi3.generated.json

@@ -7487,7 +7487,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"

templates/swagger/v1-swagger.generated.json

@@ -30599,7 +30599,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"

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())