diff --git a/models/actions/variable.go b/models/actions/variable.go index d0f917d923..39cea95c4b 100644 --- a/models/actions/variable.go +++ b/models/actions/variable.go @@ -86,7 +86,7 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab } func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) { - count, err := db.GetEngine(ctx).ID(variable.ID).Cols("name", "data"). + count, err := db.GetEngine(ctx).ID(variable.ID).Where("owner_id = ? AND repo_id = ?", variable.OwnerID, variable.RepoID).Cols("name", "data"). Update(&ActionVariable{ Name: variable.Name, Data: variable.Data, @@ -94,11 +94,9 @@ func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) return count != 0, err } -func DeleteVariable(ctx context.Context, id int64) error { - if _, err := db.DeleteByID[ActionVariable](ctx, id); err != nil { - return err - } - return nil +func DeleteVariable(ctx context.Context, variableID, ownerID, repoID int64) (bool, error) { + count, err := db.GetEngine(ctx).Table("action_variable").Where("id = ? AND owner_id = ? AND repo_id = ?", variableID, ownerID, repoID).Delete() + return count != 0, err } func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string, error) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index deff5f471e..f053a429f9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3903,6 +3903,7 @@ variables.deletion.description = Removing a variable is permanent and cannot be variables.description = Variables will be passed to certain actions and cannot be read otherwise. variables.id_not_exist = Variable with ID %d does not exist. variables.edit = Edit Variable +variables.not_found = Failed to find the variable. variables.deletion.failed = Failed to remove variable. variables.deletion.success = The variable has been removed. variables.creation.failed = Failed to add variable. diff --git a/routers/web/repo/setting/variables.go b/routers/web/repo/setting/variables.go index 45b6c0f39a..4fb8c06e84 100644 --- a/routers/web/repo/setting/variables.go +++ b/routers/web/repo/setting/variables.go @@ -127,7 +127,7 @@ func VariableUpdate(ctx *context.Context) { return } - shared.UpdateVariable(ctx, vCtx.RedirectLink) + shared.UpdateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink) } func VariableDelete(ctx *context.Context) { @@ -136,5 +136,5 @@ func VariableDelete(ctx *context.Context) { ctx.ServerError("getVariablesCtx", err) return } - shared.DeleteVariable(ctx, vCtx.RedirectLink) + shared.DeleteVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink) } diff --git a/routers/web/shared/actions/variables.go b/routers/web/shared/actions/variables.go index 79c03e4e8c..47f1176f46 100644 --- a/routers/web/shared/actions/variables.go +++ b/routers/web/shared/actions/variables.go @@ -39,25 +39,33 @@ func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL str ctx.JSONRedirect(redirectURL) } -func UpdateVariable(ctx *context.Context, redirectURL string) { +func UpdateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) { id := ctx.ParamsInt64(":variable_id") form := web.GetForm(ctx).(*forms.EditVariableForm) - if ok, err := actions_service.UpdateVariable(ctx, id, form.Name, form.Data); err != nil || !ok { - log.Error("UpdateVariable: %v", err) - ctx.JSONError(ctx.Tr("actions.variables.update.failed")) + if ok, err := actions_service.UpdateVariable(ctx, id, ownerID, repoID, form.Name, form.Data); err != nil || !ok { + if !ok { + ctx.JSONError(ctx.Tr("actions.variables.not_found")) + } else { + log.Error("UpdateVariable: %v", err) + ctx.JSONError(ctx.Tr("actions.variables.update.failed")) + } return } ctx.Flash.Success(ctx.Tr("actions.variables.update.success")) ctx.JSONRedirect(redirectURL) } -func DeleteVariable(ctx *context.Context, redirectURL string) { +func DeleteVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) { id := ctx.ParamsInt64(":variable_id") - if err := actions_service.DeleteVariableByID(ctx, id); err != nil { - log.Error("Delete variable [%d] failed: %v", id, err) - ctx.JSONError(ctx.Tr("actions.variables.deletion.failed")) + if ok, err := actions_model.DeleteVariable(ctx, id, ownerID, repoID); err != nil || !ok { + if !ok { + ctx.JSONError(ctx.Tr("actions.variables.not_found")) + } else { + log.Error("Delete variable [%d] failed: %v", id, err) + ctx.JSONError(ctx.Tr("actions.variables.deletion.failed")) + } return } ctx.Flash.Success(ctx.Tr("actions.variables.deletion.success")) diff --git a/services/actions/variables.go b/services/actions/variables.go index 8dde9c4af5..a5703898ab 100644 --- a/services/actions/variables.go +++ b/services/actions/variables.go @@ -31,7 +31,7 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data strin return v, nil } -func UpdateVariable(ctx context.Context, variableID int64, name, data string) (bool, error) { +func UpdateVariable(ctx context.Context, variableID, ownerID, repoID int64, name, data string) (bool, error) { if err := secret_service.ValidateName(name); err != nil { return false, err } @@ -41,16 +41,14 @@ func UpdateVariable(ctx context.Context, variableID int64, name, data string) (b } return actions_model.UpdateVariable(ctx, &actions_model.ActionVariable{ - ID: variableID, - Name: strings.ToUpper(name), - Data: util.ReserveLineBreakForTextarea(data), + ID: variableID, + Name: strings.ToUpper(name), + Data: util.ReserveLineBreakForTextarea(data), + OwnerID: ownerID, + RepoID: repoID, }) } -func DeleteVariableByID(ctx context.Context, variableID int64) error { - return actions_model.DeleteVariable(ctx, variableID) -} - func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name string) error { if err := secret_service.ValidateName(name); err != nil { return err @@ -69,7 +67,8 @@ func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name strin return err } - return actions_model.DeleteVariable(ctx, v.ID) + _, err = actions_model.DeleteVariable(ctx, v.ID, ownerID, repoID) + return err } func GetVariable(ctx context.Context, opts actions_model.FindVariablesOpts) (*actions_model.ActionVariable, error) {