Skip to content

Commit ca111b7

Browse files
wxiaoguangwxiaoguang
committed
fix names
1 parent 9e5888c commit ca111b7

File tree

7 files changed

+167
-17
lines changed

7 files changed

+167
-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 {

‎tests/integration/actions_variables_test.go

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

0 commit comments

Comments
 (0)