Create controllers, models, and simple views using devise gem #4
Conversation
Install Frontend Dependencies & Config React as Rails Frontend
IvanRuskevych
requested review from
rogergraves,
SerhiiBorozenets,
LyubovP and
Maryna-Harasko
app/models/user.rb
Outdated
| # validates :password, length: { minimum: 6, maximum: 20 }, presence: true, uniqueness: {case_sensitive: false} | ||
| # validates :password_confirmation, presence: true |
There was a problem hiding this comment.
Validations should be used on these
There was a problem hiding this comment.
Hi, thanks for the recommendations!
- I removed validates for password because Devise has it
- I write validates for email & password_confirmation
| # confirmation, reset password and unlock tokens in the database. | ||
| # Devise will use the `secret_key_base` as its `secret_key` | ||
| # by default. You can change it below and use your own secret key. | ||
| # config.secret_key = '706ef6afa1a8dbc795dc5806bf6bd7241c0b45ef40d92db81e067b1b7a3f5528f3075de9f7d92fc172d5e4d2b310303843bf54e9d2ba1f116fb5208a560979b8' |
There was a problem hiding this comment.
Please see this comment here
There was a problem hiding this comment.
Hi, thanks for the recommendations!
- I put config.secret_key to config/environments/test.rb & to config/environments/development.rb
app/models/user.rb
Outdated
| # :confirmable | ||
|
|
||
| # validates :password, length: { minimum: 6, maximum: 20 }, presence: true, uniqueness: {case_sensitive: false} | ||
| # validates :password_confirmation, presence: true |
There was a problem hiding this comment.
It's a good idea to add a validation for the email to ensure it follows a standard format. I recommend using the built-in regular expression URI::MailTo::EMAIL_REGEXP, which is a best practice for validating email correctness in Ruby on Rails, as it follows the standard format and simplifies the code. You should also add a validation for email uniqueness with case sensitivity to avoid duplicates.
validates :email, presence: true, uniqueness: { case_sensitive: true }, format: { with: URI::MailTo::EMAIL_REGEXP, message: "must be a valid email format" }
Don't forget about tests. You should write corresponding RSpec tests to ensure that the validations work correctly.
There was a problem hiding this comment.
Hi, thanks for the recommendations!
- I removed the validations for password because Devise already handles them.
- I added validations for email and password_confirmation.
- I plan to create tests in one of the upcoming PRs. Is that okay, or should I include them in this PR with the current implementation of the User model?
|
Hi Team, I’ve addressed the feedback provided earlier and updated the pull request accordingly. Could you please take another look when you have time? Let me know if there’s anything else I should improve. Thanks in advance! |
time: 2,5 h