Merge pull request #23296 from jrafanie/fix-undefined-ems-cluster-ems…

…_events-foreign-key

Use foreign_key on the base model's name if ems_events association doesn't exist

app/models/ems_cluster.rb

@@ -204,6 +204,12 @@ def event_where_clause(assoc = :ems_events)
     cond_params
   end
 
+  # TODO: ems_events already exists as a method but it uses
+  # event_where_clause which tacks on additional possibly expensive queries
+  # such as all events for all hosts or vms in the cluster.
+  # Should we move to looking at ems events specifically for the ems cluster?
+  # like this:
+  #   has_many :ems_events, :foreign_key => :ems_cluster_id
   def ems_events
     ewc = event_where_clause
     return [] if ewc.blank?

app/models/host_aggregate.rb

@@ -2,6 +2,9 @@ class HostAggregate < ApplicationRecord
   include SupportsFeatureMixin
   include NewWithTypeStiMixin
   include Metric::CiMixin
+
+  # TODO: this model doesn't have an event_where_clause, doesn't have a *_id in event_streams but includes
+  # event mixin.  Track down if we need timeline events for this as they look to not be completely supported yet.
   include EventMixin
   include ProviderObjectMixin
 

app/models/mixins/event_mixin.rb

@@ -63,7 +63,7 @@ def find_one_event(assoc, order)
 
   module ClassMethods
     def ems_event_filter_column
-      @ems_event_filter_column ||= reflect_on_association(:ems_events).try(:foreign_key) || name.foreign_key
+      @ems_event_filter_column ||= reflect_on_association(:ems_events).try(:foreign_key) || base_model.name.foreign_key
     end
 
     def miq_event_filter_column

app/models/physical_server_profile.rb

@@ -4,6 +4,9 @@ class PhysicalServerProfile < ApplicationRecord
   include NewWithTypeStiMixin
   include TenantIdentityMixin
   include SupportsFeatureMixin
+
+  # TODO: this model doesn't have an event_where_clause, doesn't have a *_id in event_streams but includes
+  # event mixin.  Track down if we need timeline events for this as they look to not be completely supported yet.
   include EventMixin
   include ProviderObjectMixin
   include EmsRefreshMixin

app/models/physical_switch.rb

@@ -9,7 +9,11 @@ class PhysicalSwitch < Switch
   has_one :asset_detail, :as => :resource, :dependent => :destroy, :inverse_of => :resource
   has_one :hardware, :dependent => :destroy, :foreign_key => :switch_id, :inverse_of => :physical_switch
   has_many :physical_network_ports, :dependent => :destroy, :foreign_key => :switch_id
+
+  # TODO: Deprecate event_streams if it makes sense, find callers, and change to use ems_events.  Even though event_streams
+  # have only ever been ems_events in this model, we shouldn't rely on that, so callers should use ems_events.
   has_many :event_streams, :inverse_of => :physical_switch, :dependent => :nullify
+  has_many :ems_events, :inverse_of => :physical_switch, :dependent => :nullify
 
   has_many :connected_components, :through => :physical_network_ports, :source => :connected_computer_system
 

spec/models/mixins/event_mixin_spec.rb

@@ -46,26 +46,46 @@ def event_where_clause(assoc)
     # 2) EmsCluster#event_where_clause does an OR:  ems_cluster_id OR (host_id in ? OR dest_host_id IN ?) OR (vm_or_template in ? OR dest_vm_or_template_id in ?) (for now we just do ems_cluster_id in ems_event_filter)
     # 3) Host#event_where_clause does an OR: host_id OR dest_host_id... right now we only do host_id in ems_event_filter
     # 4) VmOrTemplate#event_where_clause does an OR: vm_or_template_id OR dest_vm_or_template_id, we only do vm_or_template_id in ems_event_filter
+    # 5) The following are tested below in the custom behavior section because they have different setup:
+      # Container           container_id
+      # ContainerGroup      container_group_id
+      # ContainerNode       container_node_id
+      # ContainerProject    container_project_id
+      # ContainerReplicator container_replicator_id
+    # The following classes are missing the event_where_clause and their _id column isn't in event_streams table... fix this by removing event mixin or making it work by adding the column.
+    not_implemented = %w[HostAggregate PhysicalServerProfile]
     %w[
       AvailabilityZone    availability_zone_id
       EmsCluster          ems_cluster_id
       ExtManagementSystem ems_id
       Host                host_id
+      HostAggregate       host_aggregate_id
       PhysicalChassis     physical_chassis_id
       PhysicalServer      physical_server_id
+      PhysicalServerProfile physical_server_profile_id
       PhysicalStorage     physical_storage_id
       PhysicalSwitch      physical_switch_id
       VmOrTemplate        vm_or_template_id
       Vm                  vm_or_template_id
     ].each_slice(2) do |klass, column|
-      it "#{klass} uses #{column} and target_id and target_type" do
-        obj = FactoryBot.create(klass.tableize.singularize)
-        expect(obj.event_stream_filters["EmsEvent"]).to eq(column => obj.id)
-        expect(obj.event_stream_filters.dig("MiqEvent", "target_id")).to eq(obj.id)
-        expect(obj.event_stream_filters.dig("MiqEvent", "target_type")).to eq(obj.class.base_class.name)
+      skip = not_implemented.include?(klass)
+      klasses = klass.constantize.descendants.collect(&:name).unshift(klass)
+      klasses.each do |klass|
+        it "#{klass} uses #{column} and target_id and target_type" do
+          skip("Class: #{klass} hasn't fully implemented timeline events") if skip
+          begin
+            obj = FactoryBot.build(klass.tableize.singularize, :name => "test")
+          rescue NameError
+            skip "Unable to build factory from name: #{klass}, possibly due to inflections"
+          end
+          expect(obj.event_stream_filters["EmsEvent"]).to eq(column => obj.id)
+          expect(obj.event_stream_filters.dig("MiqEvent", "target_id")).to eq(obj.id)
+          expect(obj.event_stream_filters.dig("MiqEvent", "target_type")).to eq(obj.class.base_class.name)
+        end
       end
 
       it "#{klass} behaves like event_where_clause for ems_events" do
+        skip("Class: #{klass} hasn't implemented timeline events.  This could be due to no event_where_clause, ems_events association, and/or _id column in event_streams") if skip
         obj = FactoryBot.create(klass.tableize.singularize)
         event = FactoryBot.create(:event_stream, column => obj.id)
         FactoryBot.create(:event_stream)