From de83393487f6f11cd074dfcb19aa02e584c861e9 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 18 Jun 2026 14:02:11 +0200 Subject: [PATCH] refactor: replace legacy `delete-button` with `link-action` (#38143) Removes the legacy `delete-button` handler (`initGlobalDeleteButton`) and migrates all remaining usages to `link-action` and `show-modal` / `form-fetch-action`. Two handlers are adjusted for the new request shape: webauthn key delete reads `id` from the query, and account deletion returns `JSONError` on validation failure. A E2E test ist added to cover one of the use cases. Suggested in https://github.com/go-gitea/gitea/pull/38046#discussion_r3414936737. --------- Co-authored-by: wxiaoguang Co-authored-by: bircni --- routers/web/user/setting/account.go | 41 ++++------- routers/web/user/setting/keys.go | 29 ++++---- routers/web/user/setting/security/webauthn.go | 3 +- routers/web/web.go | 2 +- services/forms/user_form.go | 11 --- templates/org/team/members.tmpl | 16 ++--- templates/org/team/new.tmpl | 4 +- templates/repo/branch/list.tmpl | 6 +- templates/shared/actions/runner_edit.tmpl | 4 +- templates/user/settings/account.tmpl | 10 +-- templates/user/settings/applications.tmpl | 4 +- .../settings/applications_oauth2_list.tmpl | 4 +- templates/user/settings/grants_oauth2.tmpl | 4 +- templates/user/settings/keys_gpg.tmpl | 4 +- templates/user/settings/keys_principal.tmpl | 4 +- templates/user/settings/keys_ssh.tmpl | 4 +- templates/user/settings/organization.tmpl | 17 ++--- .../user/settings/security/accountlinks.tmpl | 4 +- templates/user/settings/security/openid.tmpl | 4 +- templates/user/settings/security/twofa.tmpl | 6 +- .../user/settings/security/webauthn.tmpl | 4 +- tests/e2e/org.test.ts | 18 +++++ tests/integration/branches_test.go | 8 +-- tests/integration/delete_user_test.go | 10 ++- tests/integration/html_helper.go | 1 + tests/integration/user_settings_test.go | 4 +- web_src/css/modules/button.css | 5 -- web_src/js/features/common-button.ts | 69 ------------------- web_src/js/index.ts | 3 +- 29 files changed, 108 insertions(+), 195 deletions(-) diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 1927efb7fe..2901f36284 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -242,28 +242,16 @@ func DeleteAccount(ctx *context.Context) { return } - ctx.Data["Title"] = ctx.Tr("settings_title") - ctx.Data["PageIsSettingsAccount"] = true - ctx.Data["Email"] = ctx.Doer.Email - if _, _, err := auth.UserSignIn(ctx, ctx.Doer.Name, ctx.FormString("password")); err != nil { switch { case user_model.IsErrUserNotExist(err): - loadAccountData(ctx) - - ctx.RenderWithErrDeprecated(ctx.Tr("form.user_not_exist"), tplSettingsAccount, nil) + ctx.JSONError(ctx.Tr("form.user_not_exist")) case errors.Is(err, smtp.ErrUnsupportedLoginType): - loadAccountData(ctx) - - ctx.RenderWithErrDeprecated(ctx.Tr("form.unsupported_login_type"), tplSettingsAccount, nil) + ctx.JSONError(ctx.Tr("form.unsupported_login_type")) case errors.As(err, &db.ErrUserPasswordNotSet{}): - loadAccountData(ctx) - - ctx.RenderWithErrDeprecated(ctx.Tr("form.unset_password"), tplSettingsAccount, nil) + ctx.JSONError(ctx.Tr("form.unset_password")) case errors.As(err, &db.ErrUserPasswordInvalid{}): - loadAccountData(ctx) - - ctx.RenderWithErrDeprecated(ctx.Tr("form.enterred_invalid_password"), tplSettingsAccount, nil) + ctx.JSONError(ctx.Tr("form.enterred_invalid_password")) default: ctx.ServerError("UserSignIn", err) } @@ -272,32 +260,27 @@ func DeleteAccount(ctx *context.Context) { // admin should not delete themself if ctx.Doer.IsAdmin { - ctx.Flash.Error(ctx.Tr("form.admin_cannot_delete_self")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") + ctx.JSONError(ctx.Tr("form.admin_cannot_delete_self")) return } if err := user.DeleteUser(ctx, ctx.Doer, false); err != nil { switch { case repo_model.IsErrUserOwnRepos(err): - ctx.Flash.Error(ctx.Tr("form.still_own_repo")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") + ctx.JSONError(ctx.Tr("form.still_own_repo")) case org_model.IsErrUserHasOrgs(err): - ctx.Flash.Error(ctx.Tr("form.still_has_org")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") + ctx.JSONError(ctx.Tr("form.still_has_org")) case packages_model.IsErrUserOwnPackages(err): - ctx.Flash.Error(ctx.Tr("form.still_own_packages")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") + ctx.JSONError(ctx.Tr("form.still_own_packages")) case user_model.IsErrDeleteLastAdminUser(err): - ctx.Flash.Error(ctx.Tr("auth.last_admin")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") + ctx.JSONError(ctx.Tr("auth.last_admin")) default: ctx.ServerError("DeleteUser", err) } - } else { - log.Trace("Account deleted: %s", ctx.Doer.Name) - ctx.Redirect(setting.AppSubURL + "/") + return } + + ctx.JSONRedirect(setting.AppSubURL + "/") } func loadAccountData(ctx *context.Context) { diff --git a/routers/web/user/setting/keys.go b/routers/web/user/setting/keys.go index 2e26305e4f..99323261be 100644 --- a/routers/web/user/setting/keys.go +++ b/routers/web/user/setting/keys.go @@ -247,17 +247,17 @@ func DeleteKey(ctx *context.Context) { switch ctx.FormString("type") { case "gpg": if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageGPGKeys) { - ctx.NotFound(errors.New("gpg keys setting is not allowed to be visited")) + ctx.JSONError("gpg keys setting is not allowed to be visited") return } if err := asymkey_model.DeleteGPGKey(ctx, ctx.Doer, ctx.FormInt64("id")); err != nil { - ctx.Flash.Error("DeleteGPGKey: " + err.Error()) - } else { - ctx.Flash.Success(ctx.Tr("settings.gpg_key_deletion_success")) + ctx.JSONError("Failed to delete PGP key") + return } + ctx.Flash.Success(ctx.Tr("settings.gpg_key_deletion_success")) case "ssh": if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { - ctx.NotFound(errors.New("ssh keys setting is not allowed to be visited")) + ctx.JSONError("ssh keys setting is not allowed to be visited") return } @@ -268,24 +268,23 @@ func DeleteKey(ctx *context.Context) { return } if external { - ctx.Flash.Error(ctx.Tr("settings.ssh_externally_managed")) - ctx.Redirect(setting.AppSubURL + "/user/settings/keys") + ctx.JSONError(ctx.Tr("settings.ssh_externally_managed")) return } if err := asymkey_service.DeletePublicKey(ctx, ctx.Doer, keyID); err != nil { - ctx.Flash.Error("DeletePublicKey: " + err.Error()) - } else { - ctx.Flash.Success(ctx.Tr("settings.ssh_key_deletion_success")) + ctx.JSONError("Failed to delete SSH key") + return } + ctx.Flash.Success(ctx.Tr("settings.ssh_key_deletion_success")) case "principal": if err := asymkey_service.DeletePublicKey(ctx, ctx.Doer, ctx.FormInt64("id")); err != nil { - ctx.Flash.Error("DeletePublicKey: " + err.Error()) - } else { - ctx.Flash.Success(ctx.Tr("settings.ssh_principal_deletion_success")) + ctx.JSONError("Failed to delete SSH principal key") + return } + ctx.Flash.Success(ctx.Tr("settings.ssh_principal_deletion_success")) default: - ctx.Flash.Warning("Function not implemented") - ctx.Redirect(setting.AppSubURL + "/user/settings/keys") + ctx.JSONError("unsupported key type") + return } ctx.JSONRedirect(setting.AppSubURL + "/user/settings/keys") } diff --git a/routers/web/user/setting/security/webauthn.go b/routers/web/user/setting/security/webauthn.go index a265fcdd10..4db7b47879 100644 --- a/routers/web/user/setting/security/webauthn.go +++ b/routers/web/user/setting/security/webauthn.go @@ -132,8 +132,7 @@ func WebauthnDelete(ctx *context.Context) { return } - form := web.GetForm(ctx).(*forms.WebauthnDeleteForm) - if _, err := auth.DeleteCredential(ctx, form.ID, ctx.Doer.ID); err != nil { + if _, err := auth.DeleteCredential(ctx, ctx.FormInt64("id"), ctx.Doer.ID); err != nil { ctx.ServerError("GetWebAuthnCredentialByID", err) return } diff --git a/routers/web/web.go b/routers/web/web.go index bde886177c..4525516f5a 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -643,7 +643,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Group("/webauthn", func() { m.Post("/request_register", web.Bind(forms.WebauthnRegistrationForm{}), security.WebAuthnRegister) m.Post("/register", security.WebauthnRegisterPost) - m.Post("/delete", web.Bind(forms.WebauthnDeleteForm{}), security.WebauthnDelete) + m.Post("/delete", security.WebauthnDelete) }) m.Group("/openid", func() { m.Post("", web.Bind(forms.AddOpenIDForm{}), security.OpenIDPost) diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 195c746985..75275fb5c7 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -419,17 +419,6 @@ func (f *WebauthnRegistrationForm) Validate(req *http.Request, errs binding.Erro return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// WebauthnDeleteForm for deleting WebAuthn keys -type WebauthnDeleteForm struct { - ID int64 `binding:"Required"` -} - -// Validate validates the fields -func (f *WebauthnDeleteForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { - ctx := context.GetValidateContext(req) - return middleware.Validate(errs, ctx.Data, f, ctx.Locale) -} - // PackageSettingForm form for package settings type PackageSettingForm struct { Action string diff --git a/templates/org/team/members.tmpl b/templates/org/team/members.tmpl index ac4039e596..c652d9c481 100644 --- a/templates/org/team/members.tmpl +++ b/templates/org/team/members.tmpl @@ -34,12 +34,10 @@
{{if and $.IsOrganizationOwner (not (and ($.Team.IsOwnerTeam) (eq (len $.Team.Members) 1)))}} -
- -
+ {{end}}
@@ -74,13 +72,13 @@ - + {{template "base/footer" .}} diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 7e0403a2f5..1aa306a3e9 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -176,7 +176,7 @@ {{else}} {{if not .Team.IsOwnerTeam}} - + {{end}} {{end}} @@ -187,7 +187,7 @@ -
+ -
+
+ -