Skip to content

Commit

Permalink
move next sequence calculation into miq_group
Browse files Browse the repository at this point in the history
- fix error around tenant groups with a nil sequence
- set sequence by default
- set sequence for tenant default_group
  • Loading branch information
kbrock committed Oct 28, 2015
1 parent fdb45b1 commit 1eb754c
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 17 deletions.
2 changes: 0 additions & 2 deletions app/controllers/ops_controller/ops_rbac.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1130,8 +1130,6 @@ def rbac_build_myco_tree
# Set group record variables to new values
def rbac_group_set_record_vars(group)
role = MiqUserRole.find_by_id(@edit[:new][:role])
groups = MiqGroup.all(:order => "sequence DESC")
group.sequence = groups.first.nil? ? 1 : groups.first.sequence + 1
group.description = @edit[:new][:description]
group.miq_user_role = role
group.tenant = Tenant.find_by_id(@edit[:new][:group_tenant]) if @edit[:new][:group_tenant]
Expand Down
14 changes: 3 additions & 11 deletions app/models/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class MiqGroup < ActiveRecord::Base
serialize :settings

default_value_for :group_type, USER_GROUP
default_value_for(:sequence) { next_sequence }

acts_as_miq_taggable
include ReportableMixin
Expand Down Expand Up @@ -67,17 +68,8 @@ def self.allows?(_group, _options = {})
true # Remove once this is implemented
end

def self.add(attrs)
group = new(attrs)
if group.resource.nil?
groups = where(:resource_id => nil, :resource_type => nil).order("sequence DESC")
else
groups = group.resource.miq_groups.order("sequence DESC")
end
group.sequence = groups.first.nil? ? 1 : groups.first.sequence + 1
group.save!

group
def self.next_sequence
maximum(:sequence).to_i + 1
end

def self.seed
Expand Down
2 changes: 2 additions & 0 deletions db/migrate/20151021174140_assign_tenant_default_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ def self.create_tenant_group(tenant)
create_with(
:description => "Tenant #{tenant.name} #{tenant.id} access",
:group_type => TENANT_GROUP,
:sequence => 1,
:guid => MiqUUID.new_guid,
:miq_user_role_id => role.try(:id)
).find_or_create_by!(:tenant_id => tenant.id)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
end

guid { MiqUUID.new_guid }

sequence(:sequence) # don't want to spend time looking these up
description { |g| g.role ? "EvmGroup-#{g.role}" : generate(:miq_group_description) }

after :build do |g, e|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@

t.reload
expect(t.default_miq_group_id).to be
expect(group_stub.find(t.default_miq_group_id).miq_user_role_id).to eq(tenant_role.id)
g = group_stub.find(t.default_miq_group_id)
expect(g.miq_user_role_id).to eq(tenant_role.id)
expect(g.sequence).to be
end

it "skips tenants that already have a group" do
Expand Down
46 changes: 44 additions & 2 deletions spec/models/miq_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,16 +248,18 @@
it "fails if referenced by user#current_group" do
FactoryGirl.create(:user, :miq_groups => [group])

current_count = MiqGroup.count
expect { group.destroy }.to raise_error
MiqGroup.count.should eq 1
MiqGroup.count.should eq current_count
end

it "fails if referenced by user#miq_groups" do
group2 = FactoryGirl.create(:miq_group)
FactoryGirl.create(:user, :miq_groups => [group, group2], :current_group => group2)

current_count = MiqGroup.count
expect { group.destroy }.to raise_error
MiqGroup.count.should eq 2
MiqGroup.count.should eq current_count
end

it "fails if referenced by a tenant#default_miq_group" do
Expand Down Expand Up @@ -429,4 +431,44 @@
expect(MiqGroup.non_tenant_groups).to include(g)
end
end

describe ".next_sequence" do
it "creates the first group" do
MiqGroup.delete_all
expect(MiqGroup.next_sequence).to eq(1)
end

it "detects existing groups" do
expect(MiqGroup.next_sequence).to be < 999 # sanity check
FactoryGirl.create(:miq_group, :sequence => 999)
expect(MiqGroup.next_sequence).to eq(1000)
end

it "handles nil sequences" do
MiqGroup.delete_all
g = FactoryGirl.create(:miq_group)
g.update_attribute(:sequence, nil)

expect(MiqGroup.next_sequence).to eq(1)
end

it "auto assigns a sequences" do
# don't want to get behavior from factory girl
g1 = MiqGroup.create(:description => "one")
g2 = MiqGroup.create(:description => "two")

expect(g1.sequence).to be
expect(g2.sequence).to eq(g1.sequence + 1)
end

it "builds a sequence based upon select criteria" do
expect(MiqGroup.next_sequence).to be < 999 # sanity check

FactoryGirl.create(:miq_group, :description => "want 1", :sequence => 999)
FactoryGirl.create(:miq_group, :description => "want 2", :sequence => 1000)
FactoryGirl.create(:miq_group, :description => "dont want", :sequence => 1009)

expect(MiqGroup.where("description like 'want%'").next_sequence).to eq(1001)
end
end
end

0 comments on commit 1eb754c

Please sign in to comment.