Skip to content

Commit 5f756dd

Browse files
committed
fix names
1 parent 9e5888c commit 5f756dd

File tree

7 files changed

+172
-17
lines changed

7 files changed

+172
-17
lines changed

models/actions/variable.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab
9595
return db.Find[ActionVariable](ctx, opts)
9696
}
9797

98-
func UpdateVariable(ctx context.Context, variable *ActionVariable, cols ...string) (bool, error) {
98+
func UpdateVariableCols(ctx context.Context, variable *ActionVariable, cols ...string) (bool, error) {
99+
variable.Name = strings.ToUpper(variable.Name)
99100
count, err := db.GetEngine(ctx).
100101
ID(variable.ID).
101102
Cols(cols...).

routers/api/v1/org/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
454454
v.Name = opt.Name
455455
v.Data = opt.Value
456456

457-
if _, err := actions_service.UpdateVariable(ctx, v); err != nil {
457+
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
458458
if errors.Is(err, util.ErrInvalidArgument) {
459459
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
460460
} else {

routers/api/v1/repo/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
418418
v.Name = opt.Name
419419
v.Data = opt.Value
420420

421-
if _, err := actions_service.UpdateVariable(ctx, v); err != nil {
421+
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
422422
if errors.Is(err, util.ErrInvalidArgument) {
423423
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
424424
} else {

routers/api/v1/user/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ func UpdateVariable(ctx *context.APIContext) {
216216
v.Name = opt.Name
217217
v.Data = opt.Value
218218

219-
if _, err := actions_service.UpdateVariable(ctx, v); err != nil {
219+
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
220220
if errors.Is(err, util.ErrInvalidArgument) {
221221
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
222222
} else {

routers/web/shared/actions/variables.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,16 @@ func VariableUpdate(ctx *context.Context) {
149149

150150
id := ctx.PathParamInt64("variable_id")
151151

152-
variable, ok := findVariable(ctx, id, vCtx)
153-
if !ok {
152+
variable := findActionsVariable(ctx, id, vCtx)
153+
if ctx.Written() {
154154
return
155155
}
156156

157157
form := web.GetForm(ctx).(*forms.EditVariableForm)
158158
variable.Name = form.Name
159159
variable.Data = form.Data
160160

161-
if ok, err := actions_service.UpdateVariable(ctx, variable); err != nil || !ok {
161+
if ok, err := actions_service.UpdateVariableNameData(ctx, variable); err != nil || !ok {
162162
log.Error("UpdateVariable: %v", err)
163163
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
164164
return
@@ -167,31 +167,36 @@ func VariableUpdate(ctx *context.Context) {
167167
ctx.JSONRedirect(vCtx.RedirectLink)
168168
}
169169

170-
func findVariable(ctx *context.Context, id int64, vCtx *variablesCtx) (*actions_model.ActionVariable, bool) {
170+
func findActionsVariable(ctx *context.Context, id int64, vCtx *variablesCtx) *actions_model.ActionVariable {
171171
opts := actions_model.FindVariablesOpts{
172172
IDs: []int64{id},
173173
}
174174
switch {
175175
case vCtx.IsRepo:
176176
opts.RepoID = vCtx.RepoID
177+
if opts.RepoID == 0 {
178+
panic("RepoID is 0")
179+
}
177180
case vCtx.IsOrg, vCtx.IsUser:
178181
opts.OwnerID = vCtx.OwnerID
182+
if opts.OwnerID == 0 {
183+
panic("OwnerID is 0")
184+
}
179185
case vCtx.IsGlobal:
180186
// do nothing
181187
default:
182-
ctx.ServerError("findVariable", errors.New("unable to determine"))
183-
return nil, false
188+
panic("invalid actions variable")
184189
}
185190

186191
got, err := actions_model.FindVariables(ctx, opts)
187192
if err != nil {
188193
ctx.ServerError("FindVariables", err)
189-
return nil, false
194+
return nil
190195
} else if len(got) == 0 {
191196
ctx.NotFound("FindVariables", nil)
192-
return nil, false
197+
return nil
193198
}
194-
return got[0], true
199+
return got[0]
195200
}
196201

197202
func VariableDelete(ctx *context.Context) {
@@ -203,8 +208,8 @@ func VariableDelete(ctx *context.Context) {
203208

204209
id := ctx.PathParamInt64("variable_id")
205210

206-
variable, ok := findVariable(ctx, id, vCtx)
207-
if !ok {
211+
variable := findActionsVariable(ctx, id, vCtx)
212+
if ctx.Written() {
208213
return
209214
}
210215

services/actions/variables.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data strin
3030
return v, nil
3131
}
3232

33-
func UpdateVariable(ctx context.Context, variable *actions_model.ActionVariable) (bool, error) {
33+
func UpdateVariableNameData(ctx context.Context, variable *actions_model.ActionVariable) (bool, error) {
3434
if err := secret_service.ValidateName(variable.Name); err != nil {
3535
return false, err
3636
}
@@ -41,7 +41,7 @@ func UpdateVariable(ctx context.Context, variable *actions_model.ActionVariable)
4141

4242
variable.Data = util.ReserveLineBreakForTextarea(variable.Data)
4343

44-
return actions_model.UpdateVariable(ctx, variable, "name", "data")
44+
return actions_model.UpdateVariableCols(ctx, variable, "name", "data")
4545
}
4646

4747
func DeleteVariableByID(ctx context.Context, variableID int64) error {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package integration
5+
6+
import (
7+
"context"
8+
"fmt"
9+
"net/http"
10+
"testing"
11+
12+
actions_model "code.gitea.io/gitea/models/actions"
13+
"code.gitea.io/gitea/models/db"
14+
repo_model "code.gitea.io/gitea/models/repo"
15+
"code.gitea.io/gitea/models/unittest"
16+
user_model "code.gitea.io/gitea/models/user"
17+
"code.gitea.io/gitea/tests"
18+
19+
"github.com/stretchr/testify/assert"
20+
"github.com/stretchr/testify/require"
21+
)
22+
23+
func TestActionsVariables(t *testing.T) {
24+
defer tests.PrepareTestEnv(t)()
25+
26+
ctx := context.Background()
27+
28+
require.NoError(t, db.DeleteAllRecords("action_variable"))
29+
30+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
31+
_, _ = actions_model.InsertVariable(ctx, user2.ID, 0, "VAR", "user2-var")
32+
user2Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{OwnerID: user2.ID, Name: "VAR"})
33+
userWebURL := "/user/settings/actions/variables"
34+
35+
org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
36+
_, _ = actions_model.InsertVariable(ctx, org3.ID, 0, "VAR", "org3-var")
37+
org3Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{OwnerID: org3.ID, Name: "VAR"})
38+
orgWebURL := "/org/org3/settings/actions/variables"
39+
40+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
41+
_, _ = actions_model.InsertVariable(ctx, 0, repo1.ID, "VAR", "repo1-var")
42+
repo1Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{RepoID: repo1.ID, Name: "VAR"})
43+
repoWebURL := "/user2/repo1/settings/actions/variables"
44+
45+
_, _ = actions_model.InsertVariable(ctx, 0, 0, "VAR", "global-var")
46+
globalVar := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{Name: "VAR", Data: "global-var"})
47+
adminWebURL := "/-/admin/actions/variables"
48+
49+
sessionAdmin := loginUser(t, "user1")
50+
sessionUser2 := loginUser(t, user2.Name)
51+
52+
doUpdate := func(t *testing.T, sess *TestSession, baseURL string, id int64, data string, expectedStatus int) {
53+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/%d/edit", baseURL, id), map[string]string{
54+
"_csrf": GetUserCSRFToken(t, sess),
55+
"name": "VAR",
56+
"data": data,
57+
})
58+
sess.MakeRequest(t, req, expectedStatus)
59+
}
60+
61+
doDelete := func(t *testing.T, sess *TestSession, baseURL string, id int64, expectedStatus int) {
62+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/%d/delete", baseURL, id), map[string]string{
63+
"_csrf": GetUserCSRFToken(t, sess),
64+
})
65+
sess.MakeRequest(t, req, expectedStatus)
66+
}
67+
68+
assertDenied := func(t *testing.T, sess *TestSession, baseURL string, id int64) {
69+
doUpdate(t, sess, baseURL, id, "ChangedData", http.StatusNotFound)
70+
doDelete(t, sess, baseURL, id, http.StatusNotFound)
71+
v := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: id})
72+
assert.Contains(t, v.Data, "-var")
73+
}
74+
75+
assertSuccess := func(t *testing.T, sess *TestSession, baseURL string, id int64) {
76+
doUpdate(t, sess, baseURL, id, "ChangedData", http.StatusOK)
77+
v := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: id})
78+
assert.Equal(t, "ChangedData", v.Data)
79+
doDelete(t, sess, baseURL, id, http.StatusOK)
80+
unittest.AssertNotExistsBean(t, &actions_model.ActionVariable{ID: id})
81+
}
82+
83+
t.Run("UpdateUserVar", func(t *testing.T) {
84+
theVar := user2Var
85+
t.Run("FromOrg", func(t *testing.T) {
86+
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
87+
})
88+
t.Run("FromRepo", func(t *testing.T) {
89+
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
90+
})
91+
t.Run("FromAdmin", func(t *testing.T) {
92+
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
93+
})
94+
})
95+
96+
t.Run("UpdateOrgVar", func(t *testing.T) {
97+
theVar := org3Var
98+
t.Run("FromRepo", func(t *testing.T) {
99+
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
100+
})
101+
t.Run("FromUser", func(t *testing.T) {
102+
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
103+
})
104+
t.Run("FromAdmin", func(t *testing.T) {
105+
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
106+
})
107+
})
108+
109+
t.Run("UpdateRepoVar", func(t *testing.T) {
110+
theVar := repo1Var
111+
t.Run("FromOrg", func(t *testing.T) {
112+
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
113+
})
114+
t.Run("FromUser", func(t *testing.T) {
115+
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
116+
})
117+
t.Run("FromAdmin", func(t *testing.T) {
118+
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
119+
})
120+
})
121+
122+
t.Run("UpdateGlobalVar", func(t *testing.T) {
123+
theVar := globalVar
124+
t.Run("FromOrg", func(t *testing.T) {
125+
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
126+
})
127+
t.Run("FromUser", func(t *testing.T) {
128+
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
129+
})
130+
t.Run("FromRepo", func(t *testing.T) {
131+
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
132+
})
133+
})
134+
135+
t.Run("UpdateSuccess", func(t *testing.T) {
136+
t.Run("User", func(t *testing.T) {
137+
assertSuccess(t, sessionUser2, userWebURL, user2Var.ID)
138+
})
139+
t.Run("Org", func(t *testing.T) {
140+
assertSuccess(t, sessionAdmin, orgWebURL, org3Var.ID)
141+
})
142+
t.Run("Repo", func(t *testing.T) {
143+
assertSuccess(t, sessionUser2, repoWebURL, repo1Var.ID)
144+
})
145+
t.Run("Admin", func(t *testing.T) {
146+
assertSuccess(t, sessionAdmin, adminWebURL, globalVar.ID)
147+
})
148+
})
149+
}

0 commit comments

Comments
 (0)