Skip to content

Commit

Permalink
Allow duplicate nil pxe_image_types during seed
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1588417

We want to disallow the same name template in different regions with the
same pxe_image_type.  Normally, region id sequences prevent this from
happening but we were treating nil pxe_image_types as the same.
Therefore you could have the "same" pxe_image_type in different regions
for the same name template.
  • Loading branch information
jrafanie committed Jun 11, 2018
1 parent 63f07d5 commit d379623
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
7 changes: 6 additions & 1 deletion app/models/customization_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ class CustomizationTemplate < ApplicationRecord
has_many :pxe_images, :through => :pxe_image_type

validates :pxe_image_type, :presence => true, :unless => :system?
validates :name, :unique_within_region => true
validates :name, :unique_within_region => true
validates :name, :uniqueness => {:scope => :pxe_image_type } unless :system_and_no_pxe_image_type?

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) }

def system_and_no_pxe_image_type?
system? && pxe_image_type_id.nil?
end

def self.seed_file_name
@seed_file_name ||= Rails.root.join("db", "fixtures", "#{table_name}.yml")
end
Expand Down
52 changes: 52 additions & 0 deletions spec/models/customization_template_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
describe CustomizationTemplate do
context "unique name validation" do
let(:pxe_image_type) { FactoryGirl.create(:pxe_image_type) }

it "doesn't allow same name in same region with the same pxe_image_type" do
pit = FactoryGirl.create(:pxe_image_type)
FactoryGirl.create(:customization_template, :name => "fred", :pxe_image_type => pit)
expect { FactoryGirl.create(:customization_template, :name => "fred", :pxe_image_type => pit) }.to raise_error(ActiveRecord::RecordInvalid)
end

it "doesn't allow same name in same region with the different pxe_image_type" do
FactoryGirl.create(:customization_template, :name => "fred", :pxe_image_type => FactoryGirl.create(:pxe_image_type))
expect { FactoryGirl.create(:customization_template, :name => "fred", :pxe_image_type => FactoryGirl.create(:pxe_image_type)) }.to raise_error(ActiveRecord::RecordInvalid)
end

it "allows different names with the same pxe_image_type in same region" do
FactoryGirl.create(:customization_template, :name => "fred", :pxe_image_type => pxe_image_type)
FactoryGirl.create(:customization_template, :name => "nick", :pxe_image_type => pxe_image_type)
expect(described_class.count).to eq(2)

first, second = described_class.all
expect(first.name).to_not eq(second.name)
expect(first.pxe_image_type).to eq(second.pxe_image_type)
expect(ApplicationRecord.id_to_region(first.id)).to eq(ApplicationRecord.id_to_region(second.id))
end

it "allow same name, nil pxe_image_types in different regions (for system seeding)" do
system_template = described_class.create!(:system => true, :name => "nick")
other_template_id = ApplicationRecord.id_in_region(system_template.id, ApplicationRecord.my_region_number + 1)
described_class.create!(:system => true, :name => "nick", :id => other_template_id)

first, second = described_class.all
expect(first.name).to eq(second.name)
expect(first.pxe_image_type).to be nil
expect(first.pxe_image_type).to eq(second.pxe_image_type)
expect(ApplicationRecord.id_to_region(first.id)).to_not eq(ApplicationRecord.id_to_region(second.id))
end

it "allows same name, different pxe_image_types in different regions" do
template = FactoryGirl.create(:customization_template, :name => "nick", :pxe_image_type => pxe_image_type)
other_template_id = ApplicationRecord.id_in_region(template.id, MiqRegion.my_region_number + 1)
other_pit_id = ApplicationRecord.id_in_region(pxe_image_type.id, MiqRegion.my_region_number + 1)
other_pit = FactoryGirl.create(:pxe_image_type, :id => other_pit_id)
FactoryGirl.create(:customization_template, :name => "nick", :pxe_image_type => other_pit, :id => other_template_id)

first, second = described_class.all
expect(first.name).to eq(second.name)
expect(first.pxe_image_type).to_not eq(second.pxe_image_type)
expect(ApplicationRecord.id_to_region(first.id)).to_not eq(ApplicationRecord.id_to_region(second.id))
end
end
end

0 comments on commit d379623

Please sign in to comment.