mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-31 10:22:17 +00:00
fix(api): edit and delete repository labels by name (#39974)
Repo and org label endpoints accepted a label name for GET but only a
numeric ID for PATCH and DELETE, which breaks label-sync style tools.
- PATCH and DELETE on `/repos/{owner}/{repo}/labels/{id}` and
`/orgs/{org}/labels/{id}` accept a label name when the value is not
numeric, numeric values stay IDs
- A name shared by several labels returns 400 instead of acting on an
arbitrary one
- Deleting a missing label returns 404 instead of 204
- The swagger `id` parameter of these endpoints is now a string
Fixes: https://github.com/go-gitea/gitea/issues/34937
Co-authored-by: Hello-Liar <80651389+Hello-Liar@users.noreply.github.com>
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
7 files changed
+100
-107
No files matched your search
+15
-13
@@ -331,14 +331,23 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) (
|
||||
if len(labelName) == 0 || repoID <= 0 {
|
||||
return nil, ErrRepoLabelNotExist{0, repoID}
|
||||
}
|
||||
return getUniqueLabel(ctx, builder.Eq{"name": labelName, "repo_id": repoID}, ErrRepoLabelNotExist{0, repoID})
|
||||
}
|
||||
|
||||
l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID})
|
||||
if err != nil {
|
||||
// getUniqueLabel rejects ambiguous matches because label names are not unique
|
||||
func getUniqueLabel(ctx context.Context, cond builder.Cond, errNotExist error) (*Label, error) {
|
||||
labels := make([]*Label, 0, 2)
|
||||
if err := db.GetEngine(ctx).Where(cond).Limit(2).Find(&labels); err != nil {
|
||||
return nil, err
|
||||
} else if !exist {
|
||||
return nil, ErrRepoLabelNotExist{0, repoID}
|
||||
}
|
||||
return l, nil
|
||||
switch len(labels) {
|
||||
case 0:
|
||||
return nil, errNotExist
|
||||
case 1:
|
||||
return labels[0], nil
|
||||
default:
|
||||
return nil, util.NewInvalidArgumentErrorf("label name is ambiguous, use the label id")
|
||||
}
|
||||
}
|
||||
|
||||
// GetLabelInRepoOrOrgByID returns the label with labelID scoped to the repo, falling back to the
|
||||
@@ -454,14 +463,7 @@ func GetLabelInOrgByName(ctx context.Context, orgID int64, labelName string) (*L
|
||||
if len(labelName) == 0 || orgID <= 0 {
|
||||
return nil, ErrOrgLabelNotExist{0, orgID}
|
||||
}
|
||||
|
||||
l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
} else if !exist {
|
||||
return nil, ErrOrgLabelNotExist{0, orgID}
|
||||
}
|
||||
return l, nil
|
||||
return getUniqueLabel(ctx, builder.Eq{"name": labelName, "org_id": orgID}, ErrOrgLabelNotExist{0, orgID})
|
||||
}
|
||||
|
||||
// GetLabelInOrgByID returns a label by ID in given organization.
|
||||
|
||||
@@ -264,6 +264,9 @@ func TestNewLabels(t *testing.T) {
|
||||
for _, label := range labels {
|
||||
unittest.AssertExistsAndLoadBean(t, label, unittest.Cond("id = ?", label.ID))
|
||||
}
|
||||
assert.NoError(t, issues_model.NewLabel(t.Context(), &issues_model.Label{RepoID: 2, Name: "labelName2", Color: "#000000"}))
|
||||
_, err := issues_model.GetLabelInRepoByName(t.Context(), 2, "labelName2")
|
||||
assert.ErrorIs(t, err, util.ErrInvalidArgument)
|
||||
unittest.CheckConsistencyFor(t, &issues_model.Label{}, &repo_model.Repository{})
|
||||
}
|
||||
|
||||
|
||||
+26
-33
@@ -108,7 +108,15 @@ func CreateLabel(ctx *context.APIContext) {
|
||||
ctx.JSON(http.StatusCreated, convert.ToLabel(label, nil, ctx.Org.Organization.AsUser()))
|
||||
}
|
||||
|
||||
// GetLabel get label by organization and label id
|
||||
func getOrgLabelByIDOrName(ctx *context.APIContext) (*issues_model.Label, error) {
|
||||
idOrName := ctx.PathParam("id")
|
||||
if labelID, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
|
||||
return issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, labelID)
|
||||
}
|
||||
return issues_model.GetLabelInOrgByName(ctx, ctx.Org.Organization.ID, idOrName)
|
||||
}
|
||||
|
||||
// GetLabel get label by organization and label id or name
|
||||
func GetLabel(ctx *context.APIContext) {
|
||||
// swagger:operation GET /orgs/{org}/labels/{id} organization orgGetLabel
|
||||
// ---
|
||||
@@ -123,9 +131,8 @@ func GetLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to get
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to get, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// responses:
|
||||
// "200":
|
||||
@@ -133,26 +140,13 @@ func GetLabel(ctx *context.APIContext) {
|
||||
// default:
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
var (
|
||||
label *issues_model.Label
|
||||
err error
|
||||
)
|
||||
strID := ctx.PathParam("id")
|
||||
if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil {
|
||||
label, err = issues_model.GetLabelInOrgByName(ctx, ctx.Org.Organization.ID, strID)
|
||||
} else {
|
||||
label, err = issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, intID)
|
||||
}
|
||||
l, err := getOrgLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
if issues_model.IsErrOrgLabelNotExist(err) {
|
||||
ctx.APIErrorNotFound()
|
||||
} else {
|
||||
ctx.APIErrorInternal(err)
|
||||
}
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
}
|
||||
|
||||
ctx.JSON(http.StatusOK, convert.ToLabel(label, nil, ctx.Org.Organization.AsUser()))
|
||||
ctx.JSON(http.StatusOK, convert.ToLabel(l, nil, ctx.Org.Organization.AsUser()))
|
||||
}
|
||||
|
||||
// EditLabel modify a label for an Organization
|
||||
@@ -172,9 +166,8 @@ func EditLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to edit
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to edit, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// - name: body
|
||||
// in: body
|
||||
@@ -186,13 +179,9 @@ func EditLabel(ctx *context.APIContext) {
|
||||
// default:
|
||||
// "$ref": "#/responses/error"
|
||||
form := web.GetForm[*api.EditLabelOption](ctx)
|
||||
l, err := issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id"))
|
||||
l, err := getOrgLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
if issues_model.IsErrOrgLabelNotExist(err) {
|
||||
ctx.APIErrorNotFound()
|
||||
} else {
|
||||
ctx.APIErrorInternal(err)
|
||||
}
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -235,9 +224,8 @@ func DeleteLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to delete
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to delete, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// responses:
|
||||
// "204":
|
||||
@@ -245,7 +233,12 @@ func DeleteLabel(ctx *context.APIContext) {
|
||||
// default:
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
if err := issues_model.DeleteOrgLabel(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id")); err != nil {
|
||||
l, err := getOrgLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
}
|
||||
if err := issues_model.DeleteOrgLabel(ctx, ctx.Org.Organization.ID, l.ID); err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -65,7 +65,15 @@ func ListLabels(ctx *context.APIContext) {
|
||||
ctx.JSON(http.StatusOK, convert.ToLabelList(labels, ctx.Repo.Repository, nil))
|
||||
}
|
||||
|
||||
// GetLabel get label by repository and label id
|
||||
func getRepoLabelByIDOrName(ctx *context.APIContext) (*issues_model.Label, error) {
|
||||
idOrName := ctx.PathParam("id")
|
||||
if labelID, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
|
||||
return issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, labelID)
|
||||
}
|
||||
return issues_model.GetLabelInRepoByName(ctx, ctx.Repo.Repository.ID, idOrName)
|
||||
}
|
||||
|
||||
// GetLabel get label by repository and label id or name
|
||||
func GetLabel(ctx *context.APIContext) {
|
||||
// swagger:operation GET /repos/{owner}/{repo}/labels/{id} issue issueGetLabel
|
||||
// ---
|
||||
@@ -85,9 +93,8 @@ func GetLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to get
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to get, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// responses:
|
||||
// "200":
|
||||
@@ -95,16 +102,7 @@ func GetLabel(ctx *context.APIContext) {
|
||||
// default:
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
var (
|
||||
l *issues_model.Label
|
||||
err error
|
||||
)
|
||||
strID := ctx.PathParam("id")
|
||||
if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil {
|
||||
l, err = issues_model.GetLabelInRepoByName(ctx, ctx.Repo.Repository.ID, strID)
|
||||
} else {
|
||||
l, err = issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, intID)
|
||||
}
|
||||
l, err := getRepoLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
@@ -189,9 +187,8 @@ func EditLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to edit
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to edit, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// - name: body
|
||||
// in: body
|
||||
@@ -204,7 +201,7 @@ func EditLabel(ctx *context.APIContext) {
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
form := web.GetForm[*api.EditLabelOption](ctx)
|
||||
l, err := issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
|
||||
l, err := getRepoLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
@@ -254,9 +251,8 @@ func DeleteLabel(ctx *context.APIContext) {
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the label to delete
|
||||
// type: integer
|
||||
// format: int64
|
||||
// description: id of the label to delete, or its name when not numeric
|
||||
// type: string
|
||||
// required: true
|
||||
// responses:
|
||||
// "204":
|
||||
@@ -264,7 +260,12 @@ func DeleteLabel(ctx *context.APIContext) {
|
||||
// default:
|
||||
// "$ref": "#/responses/error"
|
||||
|
||||
if err := issues_model.DeleteRepoLabel(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id")); err != nil {
|
||||
l, err := getRepoLabelByIDOrName(ctx)
|
||||
if err != nil {
|
||||
ctx.APIErrorAuto(err)
|
||||
return
|
||||
}
|
||||
if err := issues_model.DeleteRepoLabel(ctx, ctx.Repo.Repository.ID, l.ID); err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
}
|
||||
|
||||
+12
-18
@@ -14503,13 +14503,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to delete",
|
||||
"description": "id of the label to delete, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -14539,13 +14538,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to get",
|
||||
"description": "id of the label to get, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -14575,13 +14573,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to edit",
|
||||
"description": "id of the label to edit, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -26676,13 +26673,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to delete",
|
||||
"description": "id of the label to delete, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -26721,13 +26717,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to get",
|
||||
"description": "id of the label to get, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -26766,13 +26761,12 @@
|
||||
}
|
||||
},
|
||||
{
|
||||
"description": "id of the label to edit",
|
||||
"description": "id of the label to edit, or its name when not numeric",
|
||||
"in": "path",
|
||||
"name": "id",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
|
||||
+12
-18
@@ -3095,9 +3095,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to get",
|
||||
"type": "string",
|
||||
"description": "id of the label to get, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
@@ -3127,9 +3126,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to delete",
|
||||
"type": "string",
|
||||
"description": "id of the label to delete, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
@@ -3165,9 +3163,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to edit",
|
||||
"type": "string",
|
||||
"description": "id of the label to edit, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
@@ -14239,9 +14236,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to get",
|
||||
"type": "string",
|
||||
"description": "id of the label to get, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
@@ -14278,9 +14274,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to delete",
|
||||
"type": "string",
|
||||
"description": "id of the label to delete, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
@@ -14323,9 +14318,8 @@
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the label to edit",
|
||||
"type": "string",
|
||||
"description": "id of the label to edit, or its name when not numeric",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
|
||||
@@ -6,6 +6,7 @@ package integration
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -116,7 +117,7 @@ func TestAPIModifyLabels(t *testing.T) {
|
||||
assert.Equal(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color)
|
||||
|
||||
// EditLabel
|
||||
newName := "LabelNewName"
|
||||
newName := "scope/new name"
|
||||
newColor := "09876a"
|
||||
newColorWrong := "09g76a"
|
||||
req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{
|
||||
@@ -130,11 +131,16 @@ func TestAPIModifyLabels(t *testing.T) {
|
||||
Color: &newColorWrong,
|
||||
}).AddTokenAuth(token)
|
||||
MakeRequest(t, req, http.StatusUnprocessableEntity)
|
||||
nameURLStr := urlStr + "/" + url.PathEscape(newName)
|
||||
req = NewRequestWithJSON(t, "PATCH", nameURLStr, &api.EditLabelOption{}).AddTokenAuth(token)
|
||||
resp = MakeRequest(t, req, http.StatusOK)
|
||||
assert.Equal(t, dbLabel.ID, DecodeJSON(t, resp, &api.Label{}).ID)
|
||||
|
||||
// DeleteLabel
|
||||
req = NewRequest(t, "DELETE", singleURLStr).
|
||||
AddTokenAuth(token)
|
||||
req = NewRequest(t, "DELETE", nameURLStr).AddTokenAuth(token)
|
||||
MakeRequest(t, req, http.StatusNoContent)
|
||||
req = NewRequest(t, "DELETE", singleURLStr).AddTokenAuth(token)
|
||||
MakeRequest(t, req, http.StatusNotFound)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user