From d5dc9aaee7f6df738574f1ccb852bf4d8e4c886e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 22 May 2025 08:43:26 +0800 Subject: [PATCH 1/7] fix: return 201 Created for CreateVariable API responses - Change CreateVariable API response status from 204 No Content to 201 Created - Update related integration tests to expect 201 Created instead of 204 No Content Signed-off-by: Bo-Yi Wu --- routers/api/v1/org/action.go | 2 +- tests/integration/api_repo_variables_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index 700a5ef8ea852..53c4a24bb7529 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -419,7 +419,7 @@ func (Action) CreateVariable(ctx *context.APIContext) { return } - ctx.Status(http.StatusNoContent) + ctx.Status(http.StatusCreated) } // UpdateVariable update an org-level variable diff --git a/tests/integration/api_repo_variables_test.go b/tests/integration/api_repo_variables_test.go index 7847962b07d89..5f77af316b2f2 100644 --- a/tests/integration/api_repo_variables_test.go +++ b/tests/integration/api_repo_variables_test.go @@ -35,11 +35,11 @@ func TestAPIRepoVariables(t *testing.T) { }, { Name: "_", - ExpectedStatus: http.StatusNoContent, + ExpectedStatus: http.StatusCreated, }, { Name: "TEST_VAR", - ExpectedStatus: http.StatusNoContent, + ExpectedStatus: http.StatusCreated, }, { Name: "test_var", @@ -81,7 +81,7 @@ func TestAPIRepoVariables(t *testing.T) { req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{ Value: "initial_val", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusNoContent) + MakeRequest(t, req, http.StatusCreated) cases := []struct { Name string From 427fe0db729c3017a8742aa15a1277a9cddf6cfb Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 22 May 2025 09:24:06 +0800 Subject: [PATCH 2/7] fix: return 201 Created status for variable creation API - Change API response status for variable creation from 204 No Content to 201 Created - Update integration tests to expect 201 Created status after creating variables Signed-off-by: Bo-Yi Wu --- routers/api/v1/user/action.go | 2 +- tests/integration/api_repo_variables_test.go | 2 +- tests/integration/api_user_variables_test.go | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/routers/api/v1/user/action.go b/routers/api/v1/user/action.go index 04097fcc95b3f..a48508519f467 100644 --- a/routers/api/v1/user/action.go +++ b/routers/api/v1/user/action.go @@ -162,7 +162,7 @@ func CreateVariable(ctx *context.APIContext) { return } - ctx.Status(http.StatusNoContent) + ctx.Status(http.StatusCreated) } // UpdateVariable update a user-level variable which is created by current doer diff --git a/tests/integration/api_repo_variables_test.go b/tests/integration/api_repo_variables_test.go index 5f77af316b2f2..b5c88af279ae8 100644 --- a/tests/integration/api_repo_variables_test.go +++ b/tests/integration/api_repo_variables_test.go @@ -138,7 +138,7 @@ func TestAPIRepoVariables(t *testing.T) { req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{ Value: "initial_val", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusNoContent) + MakeRequest(t, req, http.StatusCreated) req = NewRequest(t, "DELETE", url).AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) diff --git a/tests/integration/api_user_variables_test.go b/tests/integration/api_user_variables_test.go index 367b83e7d4061..d430c9e21d6a1 100644 --- a/tests/integration/api_user_variables_test.go +++ b/tests/integration/api_user_variables_test.go @@ -29,11 +29,11 @@ func TestAPIUserVariables(t *testing.T) { }, { Name: "_", - ExpectedStatus: http.StatusNoContent, + ExpectedStatus: http.StatusCreated, }, { Name: "TEST_VAR", - ExpectedStatus: http.StatusNoContent, + ExpectedStatus: http.StatusCreated, }, { Name: "test_var", @@ -75,7 +75,7 @@ func TestAPIUserVariables(t *testing.T) { req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{ Value: "initial_val", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusNoContent) + MakeRequest(t, req, http.StatusCreated) cases := []struct { Name string @@ -132,7 +132,7 @@ func TestAPIUserVariables(t *testing.T) { req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{ Value: "initial_val", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusNoContent) + MakeRequest(t, req, http.StatusCreated) req = NewRequest(t, "DELETE", url).AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) From 5d9703d53b7ec0b0ea4b03cc787323f22b478bdb Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 22 May 2025 10:55:31 +0800 Subject: [PATCH 3/7] fix: return 201 Created status for variable creation requests - Change response status code from 204 No Content to 201 Created when creating a variable Signed-off-by: Bo-Yi Wu --- routers/api/v1/repo/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 237250b2c5e61..c1f9b9dd81b0a 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -373,7 +373,7 @@ func (Action) CreateVariable(ctx *context.APIContext) { return } - ctx.Status(http.StatusNoContent) + ctx.Status(http.StatusCreated) } // UpdateVariable update a repo-level variable From 104b736a07b72ab59839fe90cfeb42d9cc8ce802 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 22 May 2025 13:57:35 +0800 Subject: [PATCH 4/7] fix: refine variable creation API responses and documentation - Update variable creation responses to use repo-level terminology instead of org-level or generic descriptions - Replace 404 not found responses with 409 conflict responses when a variable name already exists - Remove redundant or outdated 204 response descriptions for variable creation endpoints Signed-off-by: Bo-Yi Wu --- routers/api/v1/org/action.go | 8 +++----- routers/api/v1/repo/action.go | 6 ++---- routers/api/v1/user/action.go | 8 +++----- templates/swagger/v1_json.tmpl | 25 ++++++++----------------- 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index 53c4a24bb7529..9c68f32962776 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -384,13 +384,11 @@ func (Action) CreateVariable(ctx *context.APIContext) { // "$ref": "#/definitions/CreateVariableOption" // responses: // "201": - // description: response when creating an org-level variable - // "204": - // description: response when creating an org-level variable + // description: response when creating a repo-level variable // "400": // "$ref": "#/responses/error" - // "404": - // "$ref": "#/responses/notFound" + // "409": + // description: variable name already exists. opt := web.GetForm(ctx).(*api.CreateVariableOption) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index c1f9b9dd81b0a..af9e9a2c73313 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -339,12 +339,10 @@ func (Action) CreateVariable(ctx *context.APIContext) { // responses: // "201": // description: response when creating a repo-level variable - // "204": - // description: response when creating a repo-level variable // "400": // "$ref": "#/responses/error" - // "404": - // "$ref": "#/responses/notFound" + // "409": + // description: variable name already exists. opt := web.GetForm(ctx).(*api.CreateVariableOption) diff --git a/routers/api/v1/user/action.go b/routers/api/v1/user/action.go index a48508519f467..cc95edb8a6190 100644 --- a/routers/api/v1/user/action.go +++ b/routers/api/v1/user/action.go @@ -127,13 +127,11 @@ func CreateVariable(ctx *context.APIContext) { // "$ref": "#/definitions/CreateVariableOption" // responses: // "201": - // description: response when creating a variable - // "204": - // description: response when creating a variable + // description: response when creating a repo-level variable // "400": // "$ref": "#/responses/error" - // "404": - // "$ref": "#/responses/notFound" + // "409": + // description: variable name already exists. opt := web.GetForm(ctx).(*api.CreateVariableOption) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index c97e52566050f..e2f99f8ee9d8a 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2259,16 +2259,13 @@ ], "responses": { "201": { - "description": "response when creating an org-level variable" - }, - "204": { - "description": "response when creating an org-level variable" + "description": "response when creating a repo-level variable" }, "400": { "$ref": "#/responses/error" }, - "404": { - "$ref": "#/responses/notFound" + "409": { + "description": "variable name already exists." } } }, @@ -5263,14 +5260,11 @@ "201": { "description": "response when creating a repo-level variable" }, - "204": { - "description": "response when creating a repo-level variable" - }, "400": { "$ref": "#/responses/error" }, - "404": { - "$ref": "#/responses/notFound" + "409": { + "description": "variable name already exists." } } }, @@ -17863,16 +17857,13 @@ ], "responses": { "201": { - "description": "response when creating a variable" - }, - "204": { - "description": "response when creating a variable" + "description": "response when creating a repo-level variable" }, "400": { "$ref": "#/responses/error" }, - "404": { - "$ref": "#/responses/notFound" + "409": { + "description": "variable name already exists." } } }, From 4bb467cef06bed6205b9717e5e159e43fe6ab478 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sun, 25 May 2025 20:31:58 +0800 Subject: [PATCH 5/7] Update routers/api/v1/user/action.go Co-authored-by: delvh --- routers/api/v1/user/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/user/action.go b/routers/api/v1/user/action.go index cc95edb8a6190..7e4ffd9403a42 100644 --- a/routers/api/v1/user/action.go +++ b/routers/api/v1/user/action.go @@ -127,7 +127,7 @@ func CreateVariable(ctx *context.APIContext) { // "$ref": "#/definitions/CreateVariableOption" // responses: // "201": - // description: response when creating a repo-level variable + // description: successfully created the user-level variable // "400": // "$ref": "#/responses/error" // "409": From ca26aa7dd27a9abd742be1e62561cd823fc27bc0 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sun, 25 May 2025 20:32:13 +0800 Subject: [PATCH 6/7] Update routers/api/v1/org/action.go Co-authored-by: delvh --- routers/api/v1/org/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index 9c68f32962776..053f27aab4080 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -384,7 +384,7 @@ func (Action) CreateVariable(ctx *context.APIContext) { // "$ref": "#/definitions/CreateVariableOption" // responses: // "201": - // description: response when creating a repo-level variable + // description: successfully created the org-level variable // "400": // "$ref": "#/responses/error" // "409": From 9db8ed068d21cf04b0b1a221498144f54371a661 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sun, 25 May 2025 21:18:01 +0800 Subject: [PATCH 7/7] docs: improve Swagger docs for variable creation error handling - Document error response for HTTP 500 when creating org and repo variables - Update Swagger to add HTTP 500 error response for variable creation endpoints - Clarify success descriptions for org-level and user-level variable creation in Swagger Signed-off-by: appleboy --- routers/api/v1/org/action.go | 2 ++ routers/api/v1/repo/action.go | 2 ++ templates/swagger/v1_json.tmpl | 10 ++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index 053f27aab4080..5cd3ea07f4d7b 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -389,6 +389,8 @@ func (Action) CreateVariable(ctx *context.APIContext) { // "$ref": "#/responses/error" // "409": // description: variable name already exists. + // "500": + // "$ref": "#/responses/error" opt := web.GetForm(ctx).(*api.CreateVariableOption) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index af9e9a2c73313..3f64e2477d799 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -343,6 +343,8 @@ func (Action) CreateVariable(ctx *context.APIContext) { // "$ref": "#/responses/error" // "409": // description: variable name already exists. + // "500": + // "$ref": "#/responses/error" opt := web.GetForm(ctx).(*api.CreateVariableOption) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index e2f99f8ee9d8a..dd2048ddbbaa6 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2259,13 +2259,16 @@ ], "responses": { "201": { - "description": "response when creating a repo-level variable" + "description": "successfully created the org-level variable" }, "400": { "$ref": "#/responses/error" }, "409": { "description": "variable name already exists." + }, + "500": { + "$ref": "#/responses/error" } } }, @@ -5265,6 +5268,9 @@ }, "409": { "description": "variable name already exists." + }, + "500": { + "$ref": "#/responses/error" } } }, @@ -17857,7 +17863,7 @@ ], "responses": { "201": { - "description": "response when creating a repo-level variable" + "description": "successfully created the user-level variable" }, "400": { "$ref": "#/responses/error"