Search code examples
ruby-on-railsvalidationruby-on-rails-5friendly-id

How do I let an AR validation happen before a module extension on my model?


I am using FriendlyID, and it creates a slug based on some attributes of a record upon creation.

  extend FriendlyId
  friendly_id :name_and_school, use: :slugged

  def name_and_school
    "#{first_name} #{last_name} #{school.name}"
  end

I also have the following on my model:

  validates :first_name, :last_name, presence: true
  validates_associated :school, presence: true

However, when I go to test this validation and submit a form with an empty value for first_name, last_name and school, I get the following error:

  Position Load (0.8ms)  SELECT "positions".* FROM "positions" WHERE 1=0
   (0.8ms)  BEGIN
   (0.6ms)  ROLLBACK
Completed 500 Internal Server Error in 51ms (ActiveRecord: 11.7ms)



NoMethodError - undefined method `name' for nil:NilClass:
  app/models/profile.rb:79:in `name_and_school'
  friendly_id (5.1.0) lib/friendly_id/slugged.rb:295:in `should_generate_new_friendly_id?'
  friendly_id (5.1.0) lib/friendly_id/slugged.rb:304:in `set_slug'

So what's clear is that it is hitting that name_and_school method, even before it hits ActiveRecord's validation checks.

If it hit the validation checks first, it wouldn't generate this error it would just reload the page with the respective errors.

So how do I fix this and make sure the FriendlyID slug is only generated if all validations pass?

Edit 1

So I tried insisting that name_and_school only returns a valid string if the values are present like so:

  def name_and_school
    "#{first_name} #{last_name} #{school.name}" if first_name.present? && last_name.present? && school.present?
  end

This now works for empty/invalid first_name and last_name attributes.

However, when I leave the school field blank, it now gives me this error:

Completed 500 Internal Server Error in 31689ms (Searchkick: 9.5ms | ActiveRecord: 23.6ms)

NoMethodError - undefined method `name' for nil:NilClass:
  app/models/profile.rb:139:in `search_data'
  searchkick (1.4.0) lib/searchkick/index.rb:289:in `search_data'
  searchkick (1.4.0) lib/searchkick/index.rb:66:in `block in bulk_index'
  searchkick (1.4.0) lib/searchkick/index.rb:66:in `bulk_index'
  searchkick (1.4.0) lib/searchkick/index.rb:54:in `store'
  searchkick (1.4.0) lib/searchkick/logging.rb:28:in `block in store'
  activesupport (5.0.0.1) lib/active_support/notifications.rb:164:in `block in instrument'
  activesupport (5.0.0.1) lib/active_support/notifications/instrumenter.rb:21:in `instrument'
  activesupport (5.0.0.1) lib/active_support/notifications.rb:164:in `instrument'
  searchkick (1.4.0) lib/searchkick/logging.rb:27:in `store'
  searchkick (1.4.0) lib/searchkick/index.rb:96:in `reindex_record'
  searchkick (1.4.0) lib/searchkick/model.rb:113:in `reindex'
  app/models/profile.rb:146:in `reindex_profile'

Which corresponds to this in my model:

after_commit :reindex_profile

  def search_data
    {
      name: name,
      bib_color: bib_color,
      height: height,
      weight: weight,
      player_type: player_type,
      school_name: school.name,
      age: age,
      position_name: positions.map(&:name)
    }
  end

  def reindex_profile
    reindex
  end

Once the new record is committed, it runs that callback -- which expects a valid school attribute.

So the nil value is still escaping the validates_associated check.

I even tried adding that same if statement to the reindex_profile method like so:

  def reindex_profile
    reindex if first_name.present? && last_name.present? && school.present? && bib_color.present? && player_type.present? && age.present?
  end

But that doesn't work either.

Edit 2

After try @codyeatworld's answer, we are making progress.

The issue now is that even though I am not getting those errors any more, the record is being created despite the validates_associated :school, presence: true not being true.

For instance, here is an example of the request:

Started POST "/profiles" for ::1 at 2016-11-13 16:32:19 -0500
Processing by ProfilesController#create as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"skJA+9NB3XhR/JLgQ==", "profile"=>{"avatar"=>#<ActionDispatch::Http::UploadedFile:0x007fbf3b35d9a8 @tempfile=#<Tempfile:/var/folders/0f/hgplttnd7d/T/RackMultipart20161113-16651-1wvdzdx.jpg>, @original_filename="Random-Image.jpg", @content_type="image/jpeg", @headers="Content-Disposition: form-data; name=\"profile[avatar]\"; filename=\"Random-Image.jpg\"\r\nContent-Type: image/jpeg\r\n">, "first_name"=>"Random", "last_name"=>"Brown", "dob(3i)"=>"13", "dob(2i)"=>"11", "dob(1i)"=>"1986", "weight"=>"", "height"=>"", "bib_color"=>"", "player_type"=>"player", "position_ids"=>[""], "school_id"=>"", "grade"=>"", "tournament_ids"=>[""], "email"=>"", "cell_phone"=>"", "home_phone"=>"", "grades_attributes"=>{"0"=>{"subject"=>"", "result"=>"", "grade_type"=>"csec", "_destroy"=>"false"}}, "transcripts_attributes"=>{"0"=>{"url_cache"=>"", "_destroy"=>"false"}}, "achievements_attributes"=>{"0"=>{"body"=>"", "achievement_type"=>"academic", "_destroy"=>"false"}}, "videos_attributes"=>{"0"=>{"vimeo_url"=>"", "official"=>"", "_destroy"=>"false"}}, "articles_attributes"=>{"0"=>{"source"=>"", "title"=>"", "url"=>"", "_destroy"=>"false"}}}, "commit"=>"Create Profile"}
  User Load (1.8ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 2], ["LIMIT", 1]]
  Role Load (1.8ms)  SELECT "roles".* FROM "roles" INNER JOIN "users_roles" ON "roles"."id" = "users_roles"."role_id" WHERE "users_roles"."user_id" = $1 AND (((roles.name = 'admin') AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))  [["user_id", 2]]
  Role Load (1.3ms)  SELECT "roles".* FROM "roles" INNER JOIN "users_roles" ON "roles"."id" = "users_roles"."role_id" WHERE "users_roles"."user_id" = $1 AND (((roles.name = 'coach') AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))  [["user_id", 2]]
  Role Load (1.3ms)  SELECT "roles".* FROM "roles" INNER JOIN "users_roles" ON "roles"."id" = "users_roles"."role_id" WHERE "users_roles"."user_id" = $1 AND (((roles.name = 'player') AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))  [["user_id", 2]]
   (1.7ms)  SELECT COUNT(*) FROM "roles" INNER JOIN "users_roles" ON "roles"."id" = "users_roles"."role_id" WHERE "users_roles"."user_id" = $1 AND (((roles.name = 'admin') AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)) OR ((roles.name = 'coach') AND (roles.resource_type IS NULL) AND (roles.resource_id IS NULL)))  [["user_id", 2]]
The "mime_type" Shrine metadata field will be set from the "Content-Type" request header, which might not hold the actual MIME type of the file. It is recommended to load the determine_mime_type plugin which determines MIME type from file content.
  Tournament Load (1.2ms)  SELECT "tournaments".* FROM "tournaments" WHERE 1=0
  Position Load (0.8ms)  SELECT "positions".* FROM "positions" WHERE 1=0
   (0.9ms)  BEGIN
  SQL (2.2ms)  INSERT INTO "profiles" ("first_name", "last_name", "dob", "bib_color", "created_at", "updated_at", "player_type", "grade", "home_phone", "cell_phone", "email", "avatar_data") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING "id"  [["first_name", "Yashin"], ["last_name", "Brown"], ["dob", Thu, 13 Nov 1986], ["bib_color", ""], ["created_at", 2016-11-13 21:32:19 UTC], ["updated_at", 2016-11-13 21:32:19 UTC], ["player_type", 0], ["grade", ""], ["home_phone", ""], ["cell_phone", ""], ["email", ""], ["avatar_data", "{\"id\":\"83d162a3d33bdc5d2527502e1d423ab3.jpg\",\"storage\":\"cache\",\"metadata\":{\"filename\":\"Random-Image.jpg\",\"size\":61798,\"mime_type\":\"image/jpeg\"}}"]]
  SQL (2.0ms)  INSERT INTO "transcripts" ("profile_id", "created_at", "updated_at") VALUES ($1, $2, $3) RETURNING "id"  [["profile_id", 30], ["created_at", 2016-11-13 21:32:19 UTC], ["updated_at", 2016-11-13 21:32:19 UTC]]
  Profile Load (3.7ms)  SELECT  "profiles".* FROM "profiles" WHERE "profiles"."id" = $1 LIMIT $2  [["id", 30], ["LIMIT", 1]]
   (1.6ms)  COMMIT
   (0.9ms)  BEGIN
  SQL (2.6ms)  UPDATE "profiles" SET "updated_at" = $1, "avatar_data" = $2 WHERE "profiles"."id" = $3  [["updated_at", 2016-11-13 21:32:19 UTC], ["avatar_data", "{\"id\":\"ec63dcc18ed5d60aa7a6626550f9f9ea.jpg\",\"storage\":\"store\",\"metadata\":{\"filename\":\"Random-Image.jpg\",\"size\":61798,\"mime_type\":\"image/jpeg\"}}"], ["id", 30]]
   (1.3ms)  COMMIT
  Profile Store (311.8ms)  {"id":30}
  Profile Store (32.2ms)  {"id":30}
  Profile Store (26.7ms)  {"id":30}
  Profile Store (18.9ms)  {"id":30}
Redirected to http://localhost:3000/profiles/30
Completed 302 Found in 530ms (Searchkick: 389.6ms | ActiveRecord: 32.2ms)

Notice that "school_id"=>"", also notice that the URL is now profiles/30 (rather than the FriendlyID URL, which means the friendlyID slug didn't execute which is what I want).

Why is this record still being created despite my validates_associated call I have on the model?


Solution

  • I think you're looking for should_generate_new_friendly_id then.

    def should_generate_new_friendly_id?
      first_name.present? && last_name.present? && school.present?
    end
    

    FriendlyId will only try to create a slug if these conditions are met.

    I think the offending line in searchkick is the school.name attribute.

    def search_data
      {
        name: name,
        bib_color: bib_color,
        height: height,
        weight: weight,
        player_type: player_type,
        # school_name: school.name,
        school_name: (school.present? ? school.name : nil),
        age: age,
        position_name: positions.map(&:name)
      }
    end
    

    I'm assuming that you want to validate the presence of school. The method validates_associated will run the validations for the school object/model. It doesn't check the presence of school_id.

    validates :first_name, :last_name, :school_id, presence: true
    # Remove validates_associated :school, presence: true
    

    I think you can also omit _id:

    belongs_to :school
    validates :school, presence: true