0

I am a little new to the Ruby language and Rails framework but I really want to add some features to a registration page. Although this code works, I need to make it simpler and less cluttered but I am not sure where to begin here.

class User < ActiveRecord::Base
  has_many :posts
  has_many :comments
  has_many :votes

  has_secure_password validations: false

  validates :username, presence: true, on: :create, length: {minimum: 5}, uniqueness: true
  validates :username, presence: true, on: :update, length: {minimum: 5}, uniqueness: true

  validates :password, presence: true, on: :create, length: {minimum: 5}
  validates :password, presence: true, on: :update, length: {minimum: 5}

  validates_format_of :username, on: :create, with: /\A[A-Za-z\d_]+\z/, message: "can only include letters and numbers"
  validates_format_of :username, on: :update, with: /\A[A-Za-z\d_]+\z/, message: "can only include letters and numbers"
end

I want my users to be able to only include letters and numbers with no spaces for both their username and password. Also, they both need to be a minimum of 5 characters. Right now, only characters and numbers with no spaces works with my create action but it does not work for my update action. Any help would be greatly appreciated.

user2045764
  • 175
  • 1
  • 1
  • 9
  • make sure to mark code as code so that other people can ready it easily – MZaragoza Sep 25 '13 at 00:02
  • 1
    One tip for removing the duplicate validations: http://stackoverflow.com/a/1393722/624590 – DRobinson Sep 25 '13 at 00:03
  • 2
    you don't need to specify actions since but default the validations run on both create and update – Fenec Sep 25 '13 at 00:06
  • 1
    While, it's true that OP has room for improvements in his code, I wouldn't say it's *too cluttered*. Most models I work with usually has 100+ LOC and some go over 1k. Fat models are bad. – Jason Kim Sep 25 '13 at 00:35
  • 1
    This is a very **small** model. The only clutter you've added is a bunch of duplicate validations. Remove the `on: :create` and `on: :update`, and just make one validation for each field. As an aside, it's a **terrible** idea to only allow "letters and numbers" in your user's *passwords*. – user229044 Sep 25 '13 at 01:54

2 Answers2

1

This is a very small model.

That said, there is lots of room for improvement, as you've introduced a ton of clutter.

  • There is no reason to specify on: :create and on: :update for two duplicate validations. If you just omit the on:, then it will automatically apply to both create and update:

    # Validate on both create AND update
validates :username, presence: true, length: { minimum: 5 }, uniquess: true

  • You can merge your format validation into the first validates line, and simplify the regex dramatically down to just \A\w*\z, since \w matches A-Za-z0-9_:

    validates :username, presence: true, length: { minimum: 5 }, uniquess: true,
                     format: { with: /\A\w*\z/ }

  • You should move the validation message into config/locals/en.yml instead of having it directly in the model. User-facing strings have absolutely no place hard-coded in your source code, they should always reside in a localization file. See the i18n docs.

    # config/locals/en.yml
activerecord:
  errors:
    models:
      user:
        attributes:
          username:
            too_short: "Your username is too short"
            too_long: "Your username is too long"

All told, your model should look like this (note you should be specifying a maximum length validation as well as a minimum):

class User < ActiveRecord::Base
  has_many :posts
  has_many :comments
  has_many :votes

  has_secure_password validations: false

  validates :username, presence: true,
                       length: { within: 5..20 },
                       uniqueness: true,
                       format: { with: /\A\w*\z/ }

  validates :password, presence: true,
                       length: { within: 5..20 }
end
user229044
  • 232,980
  • 40
  • 330
  • 338
  • Meagar that was very informative, thanks. My only issue is that the "can only include letters and numbers" is not working for both the create and update controllers. – user2045764 Sep 25 '13 at 02:15
  • @user2045764 I had the syntax of the `validates ... format:` wrong; I dropped the `with:` while editing. Now fixed. – user229044 Sep 25 '13 at 02:16
  • Still no luck, It seems good to me but the format: isn't working. WOuld you be able to tell me possibly at which layer would cause that issue because everything else works just fine – user2045764 Sep 25 '13 at 02:25
0
class User < ActiveRecord::Base
  has_many :posts
  has_many :comments
  has_many :votes

  has_secure_password validations: false

  validates :username, presence: true, length: {minimum: 5}, uniqueness: true

  validates :password, presence: true, length: {minimum: 5}

  validates_format_of :username, with: /\A[A-Za-z\d_]+\z/, message: "can only include letters and numbers"
end

if you do something like this

MZaragoza
  • 10,108
  • 9
  • 71
  • 116
  • I think we can also append the format validation to the validates :username in the first line validates :username, presence: true, length: {minimum: 5}, uniqueness: true, :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, :message => "can only include letters and numbers"}, on: [:create, :update] – Raghu Sep 25 '13 at 00:10
  • Specifying `on:` is probably only extra noise; validations run on create, update, and save by default. Skipping validation on save would be a weird (but possible, I guess) use case. – colinm Sep 25 '13 at 00:21
  • Moises that code doesn't seem to work with me. I basically copied and pasted your exact code because it makes sense to me syntax wise. I get a very long error when I submit any info to my registration page: /usr/local/rvm/rubies/ruby-2.0.0-p247/lib/ruby/gems/2.0.0/gems/activesupport-4.0.0/lib/active_support/callbacks.rb:503: syntax error, unexpected '[', expecting tSTRING_CONTENT or tSTRING_DBEG or tSTRING_DVAR or tSTRING_END ...ue && (validation_context == :[:create, :update]) – user2045764 Sep 25 '13 at 01:37
  • Moises - I'm getting an error where: 'user.rb:12: syntax error, unexpected end-of-input, expecting keyword_end'. https://gist.github.com/marcoroman89/6694204/raw/3883784b8f696a8cc64b5265a0ad259a02493753/users_controller – user2045764 Sep 25 '13 at 01:58
  • `def create @user = User.new(params[:user]) if @user.save flash[:notice] = "You have successfully registered, please log in!" redirect_to login_path else render :new end end` – MZaragoza Sep 25 '13 at 11:58
  • @user2045764 https://gist.github.com/mzararagoza/6698662 what are the values of your params? – MZaragoza Sep 25 '13 at 12:00