From ff865201301ee3438f2be76a4bc35ddea327d61d Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Thu, 17 Aug 2023 19:24:00 -0400
Subject: [PATCH] fix GuestDevice#child_device references

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 https://github.com/ManageIQ/manageiq/pull/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
```
---
 app/models/guest_device.rb       |  2 +-
 spec/models/guest_device_spec.rb | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/models/guest_device.rb b/app/models/guest_device.rb
index 717e1deaa11a..e0d6be652189 100644
--- a/app/models/guest_device.rb
+++ b/app/models/guest_device.rb
@@ -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
diff --git a/spec/models/guest_device_spec.rb b/spec/models/guest_device_spec.rb
index 43a46bf24f4e..481ad6fee5cd 100644
--- a/spec/models/guest_device_spec.rb
+++ b/spec/models/guest_device_spec.rb
@@ -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