Fix for STI scoping across leaves

app/models/customization_template.rb

@@ -5,7 +5,7 @@ class CustomizationTemplate < ApplicationRecord
  has_many :pxe_images, :through => :pxe_image_type
 
  validates :pxe_image_type, :presence => true, :unless => :system?
- validates :name, :uniqueness => { :scope => :pxe_image_type }, :unique_within_region => true
+ validates :name, :unique_within_region => true
 
  scope :with_pxe_image_type_id, ->(pxe_image_type_id) { where(:pxe_image_type_id => pxe_image_type_id) }
  scope :with_system, ->(bool = true) { where(:system => bool) }

lib/unique_within_region_validator.rb

@@ -2,11 +2,12 @@ class UniqueWithinRegionValidator < ActiveModel::EachValidator
  def validate_each(record, attribute, value)
  return if value.nil?
  match_case = options.key?(:match_case) ? options[:match_case] : true
+ record_base_class = record.class.base_class
  field_matches =
  if match_case
- record.class.where(attribute => value)
+ record_base_class.where(attribute => value)
  else
- record.class.where(record.class.arel_attribute(attribute).lower.eq(value.downcase))
+ record_base_class.where(record_base_class.arel_attribute(attribute).lower.eq(value.downcase))
  end
  unless field_matches.in_region(record.region_id).where.not(:id => record.id).empty?
  record.errors.add(attribute, "is not unique within region #{record.region_id}")

spec/factories/pxe_image_type.rb

@@ -1,4 +1,5 @@
 FactoryGirl.define do
  factory :pxe_image_type do
+ sequence(:name) { |n| "pxe_image_type_#{seq_padded_for_sorting(n)}" }
  end
 end

spec/lib/unique_within_region_validator_spec.rb

@@ -1,73 +1,92 @@
 describe UniqueWithinRegionValidator do
  describe "#unique_within_region" do
- let(:case_sensitive_class) do
- Class.new(User).tap do |c|
- c.class_eval do
- validates :name, :unique_within_region => true
+ context "class without STI" do
+ let(:case_sensitive_class) do
+ Class.new(User).tap do |c|
+ c.class_eval do
+ validates :name, :unique_within_region => true
+ end
  end
  end
- end
 
- let(:case_insensitive_class) do
- Class.new(User).tap do |c|
- c.class_eval do
- validates :name, :unique_within_region => {:match_case => false}
+ let(:case_insensitive_class) do
+ Class.new(User).tap do |c|
+ c.class_eval do
+ validates :name, :unique_within_region => {:match_case => false}
+ end
  end
  end
- end
 
- let(:test_name) { "thename" }
+  let(:test_name) { "thename" }
 
- let(:in_first_region_id) do
- FactoryGirl.create(
- :user,
- :id => case_sensitive_class.id_in_region(1, 0),
- :name => test_name
- ).id
- end
+  let(:in_first_region_id) do
+  FactoryGirl.create(
+  :user,
+  :id => case_sensitive_class.id_in_region(1, 0),
+  :name => test_name
+  ).id
+  end
 
- let(:also_in_first_region_id) do
- FactoryGirl.create(
- :user,
- :id => case_sensitive_class.id_in_region(2, 0),
- :name => test_name.upcase
- ).id
- end
+  let(:also_in_first_region_id) do
+  FactoryGirl.create(
+  :user,
+  :id => case_sensitive_class.id_in_region(2, 0),
+  :name => test_name.upcase
+  ).id
+  end
 
- let(:in_second_region_id) do
- FactoryGirl.create(
- :user,
- :id => case_sensitive_class.id_in_region(2, 1),
- :name => test_name
- ).id
- end
+  let(:in_second_region_id) do
+  FactoryGirl.create(
+  :user,
+  :id => case_sensitive_class.id_in_region(2, 1),
+  :name => test_name
+  ).id
+  end
 
- it "returns true if the field is unique for the records in the region" do
- expect(case_sensitive_class.find(in_first_region_id).valid?).to be true
- expect(case_sensitive_class.find(also_in_first_region_id).valid?).to be true
- expect(case_sensitive_class.find(in_second_region_id).valid?).to be true
- end
+  it "returns true if the field is unique for the records in the region" do
+  expect(case_sensitive_class.find(in_first_region_id).valid?).to be true
+  expect(case_sensitive_class.find(also_in_first_region_id).valid?).to be true
+  expect(case_sensitive_class.find(in_second_region_id).valid?).to be true
+  end
 
- it "returns false if the field is not unique for the records in the region" do
- in_first_region_rec = case_sensitive_class.find(in_first_region_id)
- also_in_first_region_rec = case_sensitive_class.find(also_in_first_region_id)
- in_second_region_rec = case_sensitive_class.find(in_second_region_id)
+  it "returns false if the field is not unique for the records in the region" do
+  in_first_region_rec = case_sensitive_class.find(in_first_region_id)
+  also_in_first_region_rec = case_sensitive_class.find(also_in_first_region_id)
+  in_second_region_rec = case_sensitive_class.find(in_second_region_id)
 
- also_in_first_region_rec.name = in_first_region_rec.name
+  also_in_first_region_rec.name = in_first_region_rec.name
 
- expect(in_first_region_rec.valid?).to be true
- expect(also_in_first_region_rec.valid?).to be false
- expect(in_second_region_rec.valid?).to be true
+ expect(in_first_region_rec.valid?).to be true
+ expect(also_in_first_region_rec.valid?).to be false
+ expect(in_second_region_rec.valid?).to be true
+ end
+
+ it "is case insensitive if match_case is set to false" do
+ in_first_region_rec = case_insensitive_class.find(in_first_region_id)
+ also_in_first_region_rec = case_insensitive_class.find(also_in_first_region_id)
+ in_second_region_rec = case_insensitive_class.find(in_second_region_id)
+
+ expect(in_first_region_rec.valid?).to be false
+ expect(also_in_first_region_rec.valid?).to be false
+ expect(in_second_region_rec.valid?).to be true
+ end
  end
 
- it "is case insensitive if match_case is set to false" do
- in_first_region_rec = case_insensitive_class.find(in_first_region_id)
- also_in_first_region_rec = case_insensitive_class.find(also_in_first_region_id)
- in_second_region_rec  = case_insensitive_class.find(in_second_region_id)
+ context "class with STI" do
+ context "two subclasses" do
+  it "raises error with non-unique names in same region" do
+ FactoryGirl.create(:customization_template_cloud_init, :name => "foo")
 
- expect(in_first_region_rec.valid?).to be false
- expect(also_in_first_region_rec.valid?).to be false
- expect(in_second_region_rec.valid?).to be true
+ expect { FactoryGirl.create(:customization_template_sysprep, :name => "foo") }
+ .to raise_error(ActiveRecord::RecordInvalid, / Name is not unique within region/)
+ end
+
+ it "doesn't raise error with unique names" do
+ FactoryGirl.create(:customization_template_cloud_init)
+
+ expect { FactoryGirl.create(:customization_template_sysprep) }.to_not raise_error
+ end
+ end
  end
  end
 end