From 6f50552a15270d0797328f053368ac0d850a0e72 Mon Sep 17 00:00:00 2001 From: mikestefanello Date: Fri, 24 Dec 2021 08:42:42 -0500 Subject: [PATCH] Refactored all forms to follow new pattern. --- controller/controller.go | 55 +----------------- controller/controller_test.go | 22 ------- htmx/htmx.go | 2 +- routes/contact.go | 2 - routes/forgot_password.go | 75 ++++++++++++------------ routes/login.go | 4 +- routes/register.go | 80 +++++++++++++------------- routes/reset_password.go | 67 +++++++++++---------- templates/pages/forgot-password.gohtml | 3 +- templates/pages/register.gohtml | 12 ++-- templates/pages/reset-password.gohtml | 6 +- 11 files changed, 133 insertions(+), 195 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index a460fc1..050ebd7 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -4,15 +4,10 @@ import ( "bytes" "fmt" "net/http" - "reflect" - "goweb/htmx" "goweb/middleware" - "goweb/msg" "goweb/services" - "github.com/go-playground/validator/v10" - "github.com/eko/gocache/v2/marshaler" "github.com/eko/gocache/v2/store" @@ -155,57 +150,11 @@ func (c *Controller) cachePage(ctx echo.Context, page Page, html *bytes.Buffer) // Redirect redirects to a given route name with optional route parameters func (c *Controller) Redirect(ctx echo.Context, route string, routeParams ...interface{}) error { url := ctx.Echo().Reverse(route, routeParams) - h := htmx.Response{} - h.Redirect = url - h.Apply(ctx) + // TODO: HTMX redirect? return ctx.Redirect(http.StatusFound, url) } func (c *Controller) Fail(ctx echo.Context, err error, log string) error { ctx.Logger().Errorf("%s: %v", log, err) - return echo.NewHTTPError(500) -} - -// SetValidationErrorMessages sets error flash messages for validation failures of a given struct -// and attempts to provide more user-friendly wording. -// The error should result from the validator module and the data should be the struct that failed -// validation. -// This method supports including a struct tag of "labeL" on each field which will be the name -// of the field used in the error messages, for example: -// - FirstName string `form:"first-name" validate:"required" label:"First name"` -// Only a few validator tags are supported below. Expand them as needed. -func (c *Controller) SetValidationErrorMessages(ctx echo.Context, err error, data interface{}) { - ves, ok := err.(validator.ValidationErrors) - if !ok { - return - } - - for _, ve := range ves { - var message string - - // Default the field label to the name of the struct field - label := ve.StructField() - - // Attempt to get a label from the field's struct tag - if field, ok := reflect.TypeOf(data).FieldByName(ve.Field()); ok { - if labelTag := field.Tag.Get("label"); labelTag != "" { - label = labelTag - } - } - - // Provide better error messages depending on the failed validation tag - // This should be expanded as you use additional tags in your validation - switch ve.Tag() { - case "required": - message = "%s is required." - case "email": - message = "%s must be a valid email address." - case "eqfield": - message = "%s must match." - default: - message = "%s is not a valid value." - } - - msg.Danger(ctx, fmt.Sprintf(message, ""+label+"")) - } + return echo.NewHTTPError(http.StatusInternalServerError) } diff --git a/controller/controller_test.go b/controller/controller_test.go index 7f1ec1f..149ad65 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -10,7 +10,6 @@ import ( "goweb/config" "goweb/middleware" - "goweb/msg" "goweb/services" "goweb/tests" @@ -18,8 +17,6 @@ import ( "github.com/eko/gocache/v2/marshaler" - "github.com/go-playground/validator/v10" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -56,25 +53,6 @@ func TestController_Redirect(t *testing.T) { assert.Equal(t, http.StatusFound, ctx.Response().Status) } -func TestController_SetValidationErrorMessages(t *testing.T) { - type example struct { - Name string `validate:"required" label:"Label test"` - } - e := example{} - v := validator.New() - err := v.Struct(e) - require.Error(t, err) - - ctx, _ := tests.NewContext(c.Web, "/") - tests.InitSession(ctx) - ctr := NewController(c) - ctr.SetValidationErrorMessages(ctx, err, e) - - msgs := msg.Get(ctx, msg.TypeDanger) - require.Len(t, msgs, 1) - assert.Equal(t, "Label test is required.", msgs[0]) -} - func TestController_RenderPage(t *testing.T) { setup := func() (echo.Context, *httptest.ResponseRecorder, Controller, Page) { ctx, rec := tests.NewContext(c.Web, "/test/TestController_RenderPage") diff --git a/htmx/htmx.go b/htmx/htmx.go index 681f521..cb78910 100644 --- a/htmx/htmx.go +++ b/htmx/htmx.go @@ -53,7 +53,7 @@ func GetRequest(ctx echo.Context) Request { } } -func (r *Response) Apply(ctx echo.Context) { +func (r Response) Apply(ctx echo.Context) { if r.Push != "" { ctx.Response().Header().Set(HeaderPush, r.Push) } diff --git a/routes/contact.go b/routes/contact.go index a8ed302..d8f67bc 100644 --- a/routes/contact.go +++ b/routes/contact.go @@ -46,8 +46,6 @@ func (c *Contact) Post(ctx echo.Context) error { return c.Fail(ctx, err, "unable to process form submission") } - //ctx.Set(context.FormKey, form) - if !form.Submission.HasErrors() { if err := c.Container.Mail.Send(ctx, form.Email, "Hello!"); err != nil { return c.Fail(ctx, err, "unable to send email") diff --git a/routes/forgot_password.go b/routes/forgot_password.go index 545395e..4dfb7cb 100644 --- a/routes/forgot_password.go +++ b/routes/forgot_password.go @@ -18,76 +18,79 @@ type ( } ForgotPasswordForm struct { - Email string `form:"email" validate:"required,email" label:"Email address"` + Email string `form:"email" validate:"required,email"` + Submission controller.FormSubmission } ) -func (f *ForgotPassword) Get(c echo.Context) error { - p := controller.NewPage(c) - p.Layout = "auth" - p.Name = "forgot-password" - p.Title = "Forgot password" - p.Data = ForgotPasswordForm{} +func (c *ForgotPassword) Get(ctx echo.Context) error { + page := controller.NewPage(ctx) + page.Layout = "auth" + page.Name = "forgot-password" + page.Title = "Forgot password" + page.Form = ForgotPasswordForm{} - if form := c.Get(context.FormKey); form != nil { - p.Data = form.(ForgotPasswordForm) + if form := ctx.Get(context.FormKey); form != nil { + page.Form = form.(*ForgotPasswordForm) } - return f.RenderPage(c, p) + return c.RenderPage(ctx, page) } -func (f *ForgotPassword) Post(c echo.Context) error { - fail := func(message string, err error) error { - c.Logger().Errorf("%s: %v", message, err) - msg.Danger(c, "An error occurred. Please try again.") - return f.Get(c) - } +func (c *ForgotPassword) Post(ctx echo.Context) error { + var form ForgotPasswordForm + ctx.Set(context.FormKey, &form) succeed := func() error { - c.Set(context.FormKey, nil) - msg.Success(c, "An email containing a link to reset your password will be sent to this address if it exists in our system.") - return f.Get(c) + ctx.Set(context.FormKey, nil) + msg.Success(ctx, "An email containing a link to reset your password will be sent to this address if it exists in our system.") + return c.Get(ctx) } // Parse the form values - var form ForgotPasswordForm - if err := c.Bind(&form); err != nil { - return fail("unable to parse forgot password form", err) + if err := ctx.Bind(&form); err != nil { + return c.Fail(ctx, err, "unable to parse forgot password form") } - c.Set(context.FormKey, form) - // Validate the form - if err := c.Validate(form); err != nil { - f.SetValidationErrorMessages(c, err, form) - return f.Get(c) + if err := form.Submission.Process(ctx, form); err != nil { + return c.Fail(ctx, err, "unable to process form submission") + } + + if form.Submission.HasErrors() { + return c.Get(ctx) } // Attempt to load the user - u, err := f.Container.ORM.User. + u, err := c.Container.ORM.User. Query(). Where(user.Email(form.Email)). - Only(c.Request().Context()) + Only(ctx.Request().Context()) switch err.(type) { case *ent.NotFoundError: return succeed() case nil: default: - return fail("error querying user during forgot password", err) + return c.Fail(ctx, err, "error querying user during forgot password") } // Generate the token - token, _, err := f.Container.Auth.GeneratePasswordResetToken(c, u.ID) + token, _, err := c.Container.Auth.GeneratePasswordResetToken(ctx, u.ID) if err != nil { - return fail("error generating password reset token", err) + return c.Fail(ctx, err, "error generating password reset token") } - c.Logger().Infof("generated password reset token for user %d", u.ID) + + ctx.Logger().Infof("generated password reset token for user %d", u.ID) // Email the user - // TODO: better email - err = f.Container.Mail.Send(c, u.Email, fmt.Sprintf("Go here to reset your password: %s", c.Echo().Reverse("reset_password", u.ID, token))) + body := fmt.Sprintf( + "Go here to reset your password: %s", + ctx.Echo().Reverse("reset_password", u.ID, token), + ) + ctx.Logger().Info(body) + err = c.Container.Mail.Send(ctx, u.Email, body) if err != nil { - return fail("error sending password reset email", err) + return c.Fail(ctx, err, "error sending password reset email") } return succeed() diff --git a/routes/login.go b/routes/login.go index a83d4d2..8bc2296 100644 --- a/routes/login.go +++ b/routes/login.go @@ -18,8 +18,8 @@ type ( } LoginForm struct { - Email string `form:"email" validate:"required,email" label:"Email address"` - Password string `form:"password" validate:"required" label:"Password"` + Email string `form:"email" validate:"required,email"` + Password string `form:"password" validate:"required"` Submission controller.FormSubmission } ) diff --git a/routes/register.go b/routes/register.go index 43d9298..3e38b6a 100644 --- a/routes/register.go +++ b/routes/register.go @@ -15,79 +15,77 @@ type ( } RegisterForm struct { - Name string `form:"name" validate:"required" label:"Name"` - Email string `form:"email" validate:"required,email" label:"Email address"` - Password string `form:"password" validate:"required" label:"Password"` - ConfirmPassword string `form:"password-confirm" validate:"required,eqfield=Password" label:"Confirm password"` + Name string `form:"name" validate:"required"` + Email string `form:"email" validate:"required,email"` + Password string `form:"password" validate:"required"` + ConfirmPassword string `form:"password-confirm" validate:"required,eqfield=Password"` + Submission controller.FormSubmission } ) -func (r *Register) Get(c echo.Context) error { - p := controller.NewPage(c) - p.Layout = "auth" - p.Name = "register" - p.Title = "Register" - p.Data = RegisterForm{} +func (c *Register) Get(ctx echo.Context) error { + page := controller.NewPage(ctx) + page.Layout = "auth" + page.Name = "register" + page.Title = "Register" + page.Form = RegisterForm{} - if form := c.Get(context.FormKey); form != nil { - p.Data = form.(RegisterForm) + if form := ctx.Get(context.FormKey); form != nil { + page.Form = form.(*RegisterForm) } - return r.RenderPage(c, p) + return c.RenderPage(ctx, page) } -func (r *Register) Post(c echo.Context) error { - fail := func(message string, err error) error { - c.Logger().Errorf("%s: %v", message, err) - msg.Danger(c, "An error occurred. Please try again.") - return r.Get(c) - } +func (c *Register) Post(ctx echo.Context) error { + var form RegisterForm + ctx.Set(context.FormKey, &form) // Parse the form values - var form RegisterForm - if err := c.Bind(&form); err != nil { - return fail("unable to parse form values", err) + if err := ctx.Bind(&form); err != nil { + return c.Fail(ctx, err, "unable to parse register form") } - c.Set(context.FormKey, form) - // Validate the form - if err := c.Validate(form); err != nil { - r.SetValidationErrorMessages(c, err, form) - return r.Get(c) + if err := form.Submission.Process(ctx, form); err != nil { + return c.Fail(ctx, err, "unable to process form submission") + } + + if form.Submission.HasErrors() { + return c.Get(ctx) } // Hash the password - pwHash, err := r.Container.Auth.HashPassword(form.Password) + pwHash, err := c.Container.Auth.HashPassword(form.Password) if err != nil { - return fail("unable to hash password", err) + return c.Fail(ctx, err, "unable to hash password") } // Attempt creating the user - u, err := r.Container.ORM.User. + u, err := c.Container.ORM.User. Create(). SetName(form.Name). SetEmail(form.Email). SetPassword(pwHash). - Save(c.Request().Context()) + Save(ctx.Request().Context()) switch err.(type) { case nil: - c.Logger().Infof("user created: %s", u.Name) + ctx.Logger().Infof("user created: %s", u.Name) case *ent.ConstraintError: - msg.Warning(c, "A user with this email address already exists. Please log in.") - return r.Redirect(c, "login") + msg.Warning(ctx, "A user with this email address already exists. Please log in.") + return c.Redirect(ctx, "login") default: - return fail("unable to create user", err) + return c.Fail(ctx, err, "unable to create user") } // Log the user in - err = r.Container.Auth.Login(c, u.ID) + err = c.Container.Auth.Login(ctx, u.ID) if err != nil { - c.Logger().Errorf("unable to log in: %v", err) - msg.Info(c, "Your account has been created.") - return r.Redirect(c, "login") + ctx.Logger().Errorf("unable to log in: %v", err) + msg.Info(ctx, "Your account has been created.") + return c.Redirect(ctx, "login") } - msg.Info(c, "Your account has been created. You are now logged in.") - return r.Redirect(c, "home") + msg.Info(ctx, "Your account has been created. You are now logged in.") + return c.Redirect(ctx, "home") } diff --git a/routes/reset_password.go b/routes/reset_password.go index 6dcb643..c1a1640 100644 --- a/routes/reset_password.go +++ b/routes/reset_password.go @@ -16,64 +16,69 @@ type ( } ResetPasswordForm struct { - Password string `form:"password" validate:"required" label:"Password"` - ConfirmPassword string `form:"password-confirm" validate:"required,eqfield=Password" label:"Confirm password"` + Password string `form:"password" validate:"required"` + ConfirmPassword string `form:"password-confirm" validate:"required,eqfield=Password"` + Submission controller.FormSubmission } ) -func (r *ResetPassword) Get(c echo.Context) error { - p := controller.NewPage(c) - p.Layout = "auth" - p.Name = "reset-password" - p.Title = "Reset password" - return r.RenderPage(c, p) +func (c *ResetPassword) Get(ctx echo.Context) error { + page := controller.NewPage(ctx) + page.Layout = "auth" + page.Name = "reset-password" + page.Title = "Reset password" + page.Form = ResetPasswordForm{} + + if form := ctx.Get(context.FormKey); form != nil { + page.Form = form.(*ResetPasswordForm) + } + + return c.RenderPage(ctx, page) } -func (r *ResetPassword) Post(c echo.Context) error { - fail := func(message string, err error) error { - c.Logger().Errorf("%s: %v", message, err) - msg.Danger(c, "An error occurred. Please try again.") - return r.Get(c) - } +func (c *ResetPassword) Post(ctx echo.Context) error { + var form ResetPasswordForm + ctx.Set(context.FormKey, &form) // Parse the form values - var form ResetPasswordForm - if err := c.Bind(&form); err != nil { - return fail("unable to parse forgot password form", err) + if err := ctx.Bind(&form); err != nil { + return c.Fail(ctx, err, "unable to parse password reset form") } - // Validate the form - if err := c.Validate(form); err != nil { - r.SetValidationErrorMessages(c, err, form) - return r.Get(c) + if err := form.Submission.Process(ctx, form); err != nil { + return c.Fail(ctx, err, "unable to process form submission") + } + + if form.Submission.HasErrors() { + return c.Get(ctx) } // Hash the new password - hash, err := r.Container.Auth.HashPassword(form.Password) + hash, err := c.Container.Auth.HashPassword(form.Password) if err != nil { - return fail("unable to hash password", err) + return c.Fail(ctx, err, "unable to hash password") } // Get the requesting user - usr := c.Get(context.UserKey).(*ent.User) + usr := ctx.Get(context.UserKey).(*ent.User) // Update the user - _, err = r.Container.ORM.User. + _, err = c.Container.ORM.User. Update(). SetPassword(hash). Where(user.ID(usr.ID)). - Save(c.Request().Context()) + Save(ctx.Request().Context()) if err != nil { - return fail("unable to update password", err) + return c.Fail(ctx, err, "unable to update password") } // Delete all password tokens for this user - err = r.Container.Auth.DeletePasswordTokens(c, usr.ID) + err = c.Container.Auth.DeletePasswordTokens(ctx, usr.ID) if err != nil { - return fail("unable to delete password tokens", err) + return c.Fail(ctx, err, "unable to delete password tokens") } - msg.Success(c, "Your password has been updated.") - return r.Redirect(c, "login") + msg.Success(ctx, "Your password has been updated.") + return c.Redirect(ctx, "login") } diff --git a/templates/pages/forgot-password.gohtml b/templates/pages/forgot-password.gohtml index 18437d0..12528f4 100644 --- a/templates/pages/forgot-password.gohtml +++ b/templates/pages/forgot-password.gohtml @@ -6,7 +6,8 @@
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "Email")}}
diff --git a/templates/pages/register.gohtml b/templates/pages/register.gohtml index fd65497..a9d58e9 100644 --- a/templates/pages/register.gohtml +++ b/templates/pages/register.gohtml @@ -3,25 +3,29 @@
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "Name")}}
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "Email")}}
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "Password")}}
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "ConfirmPassword")}}
diff --git a/templates/pages/reset-password.gohtml b/templates/pages/reset-password.gohtml index eabe506..16d4b8d 100644 --- a/templates/pages/reset-password.gohtml +++ b/templates/pages/reset-password.gohtml @@ -3,13 +3,15 @@
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "Password")}}
- + + {{template "field-errors" (.Form.Submission.GetFieldErrors "ConfirmPassword")}}