Skip to content

Commit

Permalink
fix GuestDevice#child_device references
Browse files Browse the repository at this point in the history
The child_devices added unneeded scope.
This scope pulled back all ids from the guest devices table.

Since the foreign_key is added to the query, this did not do any harm.
But it adds an unneeded constraint an unneeded query.

This was introduce in ManageIQ#15371
The way the PR was batted around, it looked like this was not an intentional side effect.

Before
======

```ruby
has_many :child_devices, -> { where(:parent_device_id => ids) }, :foreign_key => "parent_device_id"
has_many :child_devices, -> { where(:parent_device_id => GuestDevice.ids) }, :foreign_key => "parent_device_id"

SELECT "guest_devices".*
FROM "guest_devices"
WHERE "guest_devices"."parent_device_id" = 26000000000035
  AND "guest_devices"."parent_device_id" IN (26000000000032, 26000000000033, 26000000000034, 26000000000035, 26000000000036, 26000000000037, 26000000000038)
```

After
=====

```ruby
has_many :child_devices, :foreign_key => "parent_device_id"

SELECT "guest_devices".*
FROM "guest_devices"
WHERE "guest_devices"."parent_device_id" = 26000000000035
```
  • Loading branch information
kbrock committed Aug 18, 2023
1 parent f9d069c commit ff86520
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/models/guest_device.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class GuestDevice < ApplicationRecord
has_many :miq_scsi_targets, :dependent => :destroy

has_many :firmwares, :dependent => :destroy
has_many :child_devices, -> { where(:parent_device_id => ids) }, :foreign_key => "parent_device_id", :class_name => "GuestDevice", :dependent => :destroy
has_many :child_devices, :foreign_key => "parent_device_id", :class_name => "GuestDevice", :dependent => :destroy

has_many :physical_network_ports, :dependent => :destroy
has_many :connected_physical_switches, :through => :physical_network_ports
Expand Down
11 changes: 11 additions & 0 deletions spec/models/guest_device_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,15 @@
expect(template_gd.host).to be_nil
expect(host_gd.host).to eq(host)
end

describe "#child_device" do
it "brings back children" do
parent = FactoryBot.create(:guest_device)
child1 = FactoryBot.create(:guest_device, :parent_device_id => parent.id)
child2 = FactoryBot.create(:guest_device, :parent_device_id => parent.id)
FactoryBot.create(:guest_device) # sad path (though the let! probably created lots of those)

expect(parent.reload.child_devices).to match_array([child1, child2])
end
end
end

0 comments on commit ff86520

Please sign in to comment.