Skip to content

Commit

Permalink
Update query and error message, add check and not allow deletion for …
Browse files Browse the repository at this point in the history
…the current login group
  • Loading branch information
lgalis committed May 24, 2017
1 parent a5d4c56 commit 18e29cc
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
15 changes: 10 additions & 5 deletions app/models/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ def self.with_current_user_groups
current_user.admin_user? ? all : where(:id => current_user.miq_group_ids)
end

def single_group_users?
group_user_ids = user_ids
return false if group_user_ids.empty?
users.includes(:miq_groups).where(:id => group_user_ids).where.not(:miq_groups => {:id => id}).count != group_user_ids.size
end

private

# if this tenant is changing, make sure this is not a default group
Expand All @@ -230,14 +236,13 @@ def validate_default_tenant
end
end

def own_users
own_users = []
users.each { |user| own_users << user if user.miq_groups.count == 1 }
own_users
def current_user_group?
id == current_user_group.try(:id)
end

def ensure_can_be_destroyed
raise _("Still has users assigned.") unless own_users.empty?
raise _("The login group cannot be deleted") if current_user_group?
raise _("The group has users assigned that do not belong to any other group") if single_group_users?
raise _("A tenant default group can not be deleted") if tenant_group? && referenced_by_tenant?
raise _("A read only group cannot be deleted.") if system_group?
end
Expand Down
42 changes: 40 additions & 2 deletions spec/models/miq_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@
it "fails if referenced by user#current_group" do
FactoryGirl.create(:user, :miq_groups => [group])

expect { expect { group.destroy }.to raise_error(RuntimeError, /Still has users assigned/) }.to_not change { MiqGroup.count }
expect { expect { group.destroy }.to raise_error(RuntimeError, /The group has users assigned that do not belong to any other group/) }.to_not change { MiqGroup.count }
end

it "succeeds if referenced by multiple user#miq_groups" do
Expand All @@ -226,11 +226,18 @@
expect { group.destroy }.not_to raise_error
end

it "fails if trying to delete the current user's group" do
group2 = FactoryGirl.create(:miq_group)
user = FactoryGirl.create(:user, :miq_groups => [group, group2], :current_group => group)
User.current_user = user
expect { expect { group.destroy }.to raise_error(RuntimeError, /The login group cannot be deleted/) }.to_not change { MiqGroup.count }
end

it "fails if one user in the group does not belong to any other group" do
group2 = FactoryGirl.create(:miq_group)
FactoryGirl.create(:user, :miq_groups => [group, group2], :current_group => group2)
FactoryGirl.create(:user, :miq_groups => [group], :current_group => group)
expect { expect { group.destroy }.to raise_error(RuntimeError, /Still has users assigned/) }.to_not change { MiqGroup.count }
expect { expect { group.destroy }.to raise_error(RuntimeError, /The group has users assigned that do not belong to any other group/) }.to_not change { MiqGroup.count }
end

it "fails if referenced by a tenant#default_miq_group" do
Expand Down Expand Up @@ -460,4 +467,35 @@
expect(group.user_count).to eq(2)
end
end

describe "#single_group_users?" do
it "returns false if all users in the group belong to an additional group" do
testgroup1 = FactoryGirl.create(:miq_group)
testgroup2 = FactoryGirl.create(:miq_group)
testgroup3 = FactoryGirl.create(:miq_group)
FactoryGirl.create(:user, :miq_groups => [testgroup1, testgroup2])
FactoryGirl.create(:user, :miq_groups => [testgroup1, testgroup3])
expect(testgroup1.single_group_users?).to eq(false)
end

it "returns false if the group has no users" do
testgroup1 = FactoryGirl.create(:miq_group)
expect(testgroup1.single_group_users?).to eq(false)
end

it "returns true if all users in a group do not belong to any other group" do
testgroup1 = FactoryGirl.create(:miq_group)
FactoryGirl.create(:user, :miq_groups => [testgroup1])
FactoryGirl.create(:user, :miq_groups => [testgroup1])
expect(testgroup1.single_group_users?).to eq(true)
end

it "returns true if any user in a group does not belong to another group" do
testgroup1 = FactoryGirl.create(:miq_group)
testgroup2 = FactoryGirl.create(:miq_group)
FactoryGirl.create(:user, :miq_groups => [testgroup1])
FactoryGirl.create(:user, :miq_groups => [testgroup1, testgroup2])
expect(testgroup1.single_group_users?).to eq(true)
end
end
end
2 changes: 1 addition & 1 deletion spec/models/miq_widget_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
end

it "the belong to group is being deleted" do
expect { group.destroy }.to raise_error(RuntimeError, /Still has users assigned/)
expect { group.destroy }.to raise_error(RuntimeError, /The group has users assigned that do not belong to any other group/)
expect(MiqWidgetSet.count).to eq(2)
end

Expand Down

0 comments on commit 18e29cc

Please sign in to comment.