Merge pull request #15041 from lgalis/allow_not_empty_group_deletion

Allow deletion of groups with users belonging to other groups
(cherry picked from commit 828b50b)

https://bugzilla.redhat.com/show_bug.cgi?id=1460307

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

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

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