Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show Specific Error Messages in New User View #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

americanhanko
Copy link
Collaborator

This PR

  • Renders the specific error messages provided by ActiveRecord in the new users view
  • Specifies Node 14 as a requirement in the README since node-sass doesn't support Node 16

Why

Previously, any error encountered during sign-up resulted in "Please fill out all fields". Now, users will receive specific feedback about what is wrong with their field entry.

Screen Shot 2021-05-30 at 7 20 44 PM
Screen Shot 2021-05-30 at 7 20 56 PM
Screen Shot 2021-05-30 at 7 21 09 PM
Screen Shot 2021-05-30 at 7 21 48 PM

@americanhanko americanhanko changed the title Eh email in use Show Specific Error Messages in New User View May 31, 2021
Comment on lines +8 to +9
* Ruby 2.7.3
* Yarn with Node.js v14
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind pulling these out into a separate PR?

Comment on lines +6 to +8
<% @user.errors.full_messages.each do |error_message| %>
<%= error_message %>
<% end %>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, but I wonder if we want to exclude the error message that says the email address is already taken. It feels like it might be responsible not to expose that information to anyone trying to use the sign up form. I think there are a few ways we could do this:

  1. We can pair the errors with their respective fields (e.g., errors for first name next to the first name field and so on).
  2. We might be able to filter out that error in particular.
  3. We can stick to the original generic error, and modify it a bit:
All fields are required. If you're having trouble, make sure you don't already have an account with the same email. Reset your password here

And we could link them to the password reset form in that message.

I'm partial to 1 or 2, but 3 is super easy. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants