Skip to content

Conversation

yshmarov
Copy link

@yshmarov yshmarov commented Jul 21, 2024

#52328 adds sessions (existing user can sign in).

This PR adds registrations (new user can sign up).

We already have database uniqueness validation for email_address, now we add validations in the User model to display errors during registration.


I think the next step to make this new authentication solution viable for a real app would be adding "forgot/reset password" feature.

@rails-bot rails-bot bot added the railties label Jul 21, 2024
@skipkayhil
Copy link
Member

The filename was changed in #52435, can you please rebase?

@dhh
Copy link
Member

dhh commented Jul 31, 2024

Appreciate the work here, but I don't like the idea of a top-level registrations controller. Maybe it's just the naming, but most apps have a great degree of variety there. There are SignupControllers or AccountControllers or UserControllers. Will think about this a bit more on how it might fit in.

Also don't like using validations for error handling. HTML has finally given us email validation straight on the input tag. Don't want to replicate that.

@dhh dhh closed this Jul 31, 2024
@yshmarov
Copy link
Author

yshmarov commented Aug 4, 2024

Appreciate the work here, but I don't like the idea of a top-level registrations controller. Maybe it's just the naming, but most apps have a great degree of variety there. There are SignupControllers or AccountControllers or UserControllers. Will think about this a bit more on how it might fit in.

Also don't like using validations for error handling. HTML has finally given us email validation straight on the input tag. Don't want to replicate that.

I do use email_field to leverage the HTML email validation.

I view the model validation as an additional safety factor against malicious users that try to manipulate the data submitted in the form :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants