Merge pull request #17930 from kbrock/bz_1623464

fix: with_role_excluding has bad subquery

app/models/miq_group.rb

@@ -61,7 +61,7 @@ def settings=(new_settings)
   end
 
   def self.with_roles_excluding(identifier)
-    where.not(:id => MiqGroup.joins(:miq_product_features)
+    where.not(:id => MiqGroup.unscope(:select).joins(:miq_product_features)
                              .where(:miq_product_features => {:identifier => identifier})
                              .select(:id))
   end

app/models/miq_user_role.rb

@@ -65,7 +65,7 @@ def limited_self_service?
   end
 
   def self.with_roles_excluding(identifier)
-    where.not(:id => MiqUserRole.joins(:miq_product_features)
+    where.not(:id => MiqUserRole.unscope(:select).joins(:miq_product_features)
                                 .where(:miq_product_features => {:identifier => identifier})
                                 .select(:id))
   end

app/models/user.rb

@@ -49,10 +49,10 @@ class User < ApplicationRecord
   serialize     :settings, Hash   # Implement settings column as a hash
   default_value_for(:settings) { Hash.new }
 
-  scope :with_same_userid, ->(id) { where(:userid => User.find(id).userid) }
+  scope :with_same_userid, ->(id) { where(:userid => User.where(:id => id).pluck(:userid)) }
 
   def self.with_roles_excluding(identifier)
-    where.not(:id => User.joins(:miq_groups => :miq_product_features)
+    where.not(:id => User.unscope(:select).joins(:miq_groups => :miq_product_features)
                              .where(:miq_product_features => {:identifier => identifier})
                              .select(:id))
   end

spec/lib/rbac/filterer_spec.rb

@@ -888,7 +888,7 @@ def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filt
     def get_rbac_results_for_and_expect_objects(klass, expected_objects)
       User.current_user = user
 
-      results = described_class.search(:class => klass).first
+      results = described_class.search(:targets => klass).first
       expect(results).to match_array(expected_objects)
     end
 
@@ -1015,14 +1015,14 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)
 
         it 'can see all roles except for EvmRole-super_administrator' do
           expect(MiqUserRole.count).to eq(3)
-          get_rbac_results_for_and_expect_objects(MiqUserRole, [tenant_administrator_user_role, user_role])
+          get_rbac_results_for_and_expect_objects(MiqUserRole.select(:id, :name), [tenant_administrator_user_role, user_role])
         end
 
         it 'can see all groups except for group with role EvmRole-super_administrator' do
           expect(MiqUserRole.count).to eq(3)
           default_group_for_tenant = user.current_tenant.miq_groups.where(:group_type => "tenant").first
           super_admin_group
-          get_rbac_results_for_and_expect_objects(MiqGroup, [group, other_group, default_group_for_tenant])
+          get_rbac_results_for_and_expect_objects(MiqGroup.select(:id, :description), [group, other_group, default_group_for_tenant])
         end
 
         it 'can see all groups in the current tenant only' do

spec/models/miq_group_spec.rb

@@ -583,4 +583,14 @@
       expect(virtual_column_sql_value(MiqGroup.where(:id => group.id), "miq_user_role_name")).to eq("EvmRole-the_role")
     end
   end
+
+  describe ".with_roles_excluding" do
+    it "handles multiple columns" do
+      a = FactoryGirl.create(:miq_group, :features => "good")
+      FactoryGirl.create(:miq_group, :features => %w(good everything))
+      FactoryGirl.create(:miq_group, :features => "everything")
+
+      expect(MiqGroup.select(:id, :description).with_roles_excluding("everything")).to match_array([a])
+    end
+  end
 end

spec/models/miq_user_role_spec.rb

@@ -261,4 +261,14 @@
       expect(role.group_count).to eq(2)
     end
   end
+
+  describe ".with_roles_excluding" do
+    it "handles multiple columns" do
+      a = FactoryGirl.create(:miq_user_role, :features => "good")
+      FactoryGirl.create(:miq_user_role, :features => %w(good everything))
+      FactoryGirl.create(:miq_user_role, :features => "everything")
+
+      expect(MiqUserRole.select(:id, :name).with_roles_excluding("everything")).to match_array([a])
+    end
+  end
 end

spec/models/user_spec.rb

@@ -585,4 +585,28 @@
       expect(User.authorize_user("admin")).to be_nil
     end
   end
+
+  describe ".with_same_userid" do
+    # this is testing the select does not break and in general, the scope works
+    it "properly handles select clause" do
+      u = FactoryGirl.create(:user)
+      expect(User.select(:id, :email).with_same_userid(u.id)).to eq([u])
+    end
+  end
+
+  describe ".with_roles_excluding" do
+    it "handles multiple columns" do
+      a1 = FactoryGirl.create(:miq_group, :features => "good")
+      a2 = FactoryGirl.create(:miq_group, :features => "something")
+      b = FactoryGirl.create(:miq_group, :features => %w(good everything))
+      c = FactoryGirl.create(:miq_group, :features => "everything")
+
+      u1 = FactoryGirl.create(:user, :miq_groups => [a1])
+      u2 = FactoryGirl.create(:user, :miq_groups => [a1, a2])
+      FactoryGirl.create(:user, :miq_groups => [a1, b])
+      FactoryGirl.create(:user, :miq_groups => [c])
+
+      expect(User.with_roles_excluding("everything").select(:id, :name)).to match_array([u1, u2])
+    end
+  end
 end