diff --git a/app/models/miq_group.rb b/app/models/miq_group.rb index 9cd63d11958..b83e4b1ed25 100644 --- a/app/models/miq_group.rb +++ b/app/models/miq_group.rb @@ -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 @@ -230,8 +236,13 @@ def validate_default_tenant end end + def current_user_group? + id == current_user_group.try(:id) + end + def ensure_can_be_destroyed - raise _("Still has users assigned.") unless 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 diff --git a/spec/models/miq_group_spec.rb b/spec/models/miq_group_spec.rb index cf04b486357..0b161f6d8ba 100644 --- a/spec/models/miq_group_spec.rb +++ b/spec/models/miq_group_spec.rb @@ -217,18 +217,27 @@ 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 "fails if referenced by user#miq_groups" do + it "succeeds if referenced by multiple user#miq_groups" do group2 = FactoryGirl.create(:miq_group) FactoryGirl.create(:user, :miq_groups => [group, group2], :current_group => group2) + expect { group.destroy }.not_to raise_error + end - expect { - expect { group.destroy }.to raise_error(RuntimeError, /Still has users assigned/) - }.to_not change { MiqGroup.count } + 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, /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 @@ -458,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 diff --git a/spec/models/miq_widget_set_spec.rb b/spec/models/miq_widget_set_spec.rb index 9a400591ba8..e702e3b0436 100644 --- a/spec/models/miq_widget_set_spec.rb +++ b/spec/models/miq_widget_set_spec.rb @@ -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