From 6dd942b66aa0f67a74acfc181ddd3c293bd8fdb7 Mon Sep 17 00:00:00 2001
From: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date: Wed, 24 May 2017 13:48:04 -0400
Subject: [PATCH] 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 828b50b82e5a7dedb2394e4db942bf1357360e6c)

https://bugzilla.redhat.com/show_bug.cgi?id=1322396
---
 app/models/miq_group.rb            | 13 ++++++-
 spec/models/miq_group_spec.rb      | 54 ++++++++++++++++++++++++++----
 spec/models/miq_widget_set_spec.rb |  2 +-
 3 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/app/models/miq_group.rb b/app/models/miq_group.rb
index 739ae5b09f6..d0bec5d4c12 100644
--- a/app/models/miq_group.rb
+++ b/app/models/miq_group.rb
@@ -224,6 +224,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
@@ -234,8 +240,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