From 09f654b1ea2a7f7c759f4af199631969233adc88 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Mon, 16 Dec 2024 17:12:53 -0500 Subject: [PATCH] Use foreign_key on the base model's name if the associations don't exist EmsCluster was properly having ems_cluster_id as the ems_event_filter_column, subclasses had cluster_id as the column. --- app/models/ems_cluster.rb | 6 +++++ app/models/mixins/event_mixin.rb | 5 +++- spec/models/mixins/event_mixin_spec.rb | 32 +++++++++++++++----------- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/app/models/ems_cluster.rb b/app/models/ems_cluster.rb index 9666cd76df0..9ae839bbf80 100644 --- a/app/models/ems_cluster.rb +++ b/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? diff --git a/app/models/mixins/event_mixin.rb b/app/models/mixins/event_mixin.rb index cadeac1fb68..130b043e7c9 100644 --- a/app/models/mixins/event_mixin.rb +++ b/app/models/mixins/event_mixin.rb @@ -63,7 +63,10 @@ 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) || + reflect_on_association(:event_streams).try(:foreign_key) || # All Events are Ems Events for some classes right now + base_model.name.foreign_key # Fallback to using the base model's rails' String#foreign_key end def miq_event_filter_column diff --git a/spec/models/mixins/event_mixin_spec.rb b/spec/models/mixins/event_mixin_spec.rb index 95ac5b2bf50..b84dbd1b299 100644 --- a/spec/models/mixins/event_mixin_spec.rb +++ b/spec/models/mixins/event_mixin_spec.rb @@ -58,20 +58,26 @@ def event_where_clause(assoc) 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) - end - - it "#{klass} behaves like event_where_clause for ems_events" do - obj = FactoryBot.create(klass.tableize.singularize) - event = FactoryBot.create(:event_stream, column => obj.id) - FactoryBot.create(:event_stream) - expect(EventStream.where(obj.event_stream_filters["EmsEvent"]).to_a).to eq([event]) - expect(EventStream.where(obj.event_where_clause(:ems_events)).to_a).to eq([event]) + klasses = klass.constantize.descendants.collect(&:name).unshift(klass) + klasses.each do |klass| + it "#{klass} uses #{column} and target_id and target_type" do + 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 + obj = FactoryBot.create(klass.tableize.singularize) + event = FactoryBot.create(:event_stream, column => obj.id) + FactoryBot.create(:event_stream) + expect(EventStream.where(obj.event_stream_filters["EmsEvent"]).to_a).to eq([event]) + expect(EventStream.where(obj.event_where_clause(:ems_events)).to_a).to eq([event]) + end # # TODO: some classes don't have this implemented or don't have columns for this # # Do we consolidate policy events and miq events?