Skip to content

Commit

Permalink
Use foreign_key on the base model's name if the associations don't exist
Browse files Browse the repository at this point in the history
EmsCluster was properly having ems_cluster_id as the ems_event_filter_column,
subclasses had cluster_id as the column.
  • Loading branch information
jrafanie committed Jan 9, 2025
1 parent d4f76b2 commit 09f654b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
6 changes: 6 additions & 0 deletions app/models/ems_cluster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
5 changes: 4 additions & 1 deletion app/models/mixins/event_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 19 additions & 13 deletions spec/models/mixins/event_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down

0 comments on commit 09f654b

Please sign in to comment.