Custom Validations Issue

I have a custom validator that I would like to protect some data on my Users to Sites has_many through relaitonship. There is one extra piece of data on the sites_users join table is_default which is type boolean with the intention being to allow each user to have exactly one default site from the list of related sites for that user.

Instead of testing the current form instance, I am getting results in my validator to these two methods based on the prevesiously saved db instance for this same record. So if the user I am editing has one related site, but the current form isntance would remove all related sites the validation will still pass, even thought I am attempting to ensure at least one site present. Also, if the user I am editing has only one default_site, but the current form instance would set three more default sites, this passes as well, even though I am trying to validate that exactly one site is listed as default.

I have included my user validator and model code below. I can't quite figure out how to validate against the form instance isntead of the previously saved instance. I would really appreciate any help anyone could provide. Thanks you!

Here is my custom validator:

** user_validator.rb**

class UserValidator < ActiveModel::Validator
  def validate(record)
    if record.sites.count < 1
      record.errors.add(:site, "must have at least one site available")
    end

    if default_site.count != 1
      record.errors.add(:site, "must have exactly one site selected as default")
    end
  end
end

and this is my user model
** user.rb**

class User < ApplicationRecord
    has_many :sites_users
    has_many :sites, through: :sites_users
    accepts_nested_attributes_for :sites_users, allow_destroy: true
    ...
    def default_site
      sites.joins(:users).where(sites_users: { is_default: true } )
    end
    ...
end

Hey Thomas,

The main issue here is your default_site method I believe. Here's the problem:

Your default_site method issues a database query.

When your form submits, you actually are modifying the record (and nested records) in memory. When your validation runs, it needs to check against the in-memory nested records that you're submitting, not the ones in the database.

class UserValidator < ActiveModel::Validator
  def validate(record)
    if record.sites.count < 1
      record.errors.add(:site, "must have at least one site available")
    end

    if record.sites_users.select{ |su| su.is_default }.count != 1
      record.errors.add(:site, "must have exactly one site selected as default")
    end
  end
end

Changing it to this will do a check against the sites_users in-memory. You can leave your database query for default site as it does some extra loading for performance elsewhere I'm assuming.

Chris,

That solves the single default for each user, totally clears up my confusion, too thanks.

Oddly though, the record.sites.count < 1 allows me to delete all sites for a users and save without validation error. This feels like it should contain the in-memory instance, what am I missing?

Probably the same issue.

Remember that count, size, and length in ActiveRecord are all different. Count always queries the database if I remember right. Size uses the counter cache if it can, but will count the in-memory items if they're loaded. Length always counts them in Ruby in-memory.

You probably want to use either size or length there to make sure you're counting the Ruby array, not the db query. (It would also probably be good to comment these nuances alongside the code so when this changes in the future you remember why you did these seemingly weird things.)

Reference:
https://mensfeld.pl/2014/09/activerecord-count-vs-length-vs-size-and-what-will-happen-if-you-use-it-the-way-you-shouldnt/

Chris,

Thanks for your help!!! I have included my final solution below on the off chance it could help someone else. Ended up discovering one more edge case, the user has two sites marked as default and deletes one of them -- the select call would still trigger a validation error.

After a bit of searching I found .marked_for_destruction? method which is really useful for nested attribute scenarios to validate against only the in-memory data a user intends to keep, instead of all in-memory data which would obviously include those records marked for deletion with "_destroy"=>"true" params

class UserValidator < ActiveModel::Validator
  def validate(record)
    if record.sites_users.select { |su| su.marked_for_destruction? == false }.count < 1
      record.errors.add(:site, "must have at least one site available")
    end

    if record.sites_users.select{ |su| su.marked_for_destruction? == false && su.is_default }.count != 1
      record.errors.add(:site, "must have exactly one site selected as default")
    end
  end
end

Thank you!

Ah yes, that would break it too!

A small refactoring would be:

class UserValidator < ActiveModel::Validator
  def validate(record)
      site_users = record.sites_users.select { |su| su.marked_for_destruction? == false }

    if site_users.count < 1
      record.errors.add(:site, "must have at least one site available")
    end

    if sites_users.select(&:is_default).count != 1
      record.errors.add(:site, "must have exactly one site selected as default")
    end
  end
end

Even better, thanks again!