Fixes #37123 - Correct virtual_host subscription facet name in Legacy UI

[chris1984]

What are the changes introduced in this pull request?

• We must have changed the name of the subscription_facet_attributes.virtual_host from display_name to name when we were doing stuff in the new UI, because it works fine there but we didn't check the old UI. This fixes that up, so it shows correctly the displays the guest/host mapping.

Considerations taken when implementing this change?

• N/A

What are the testing steps for this pull request?

• Spin up a 6.15/stream in beaker, need to be on the RH network so you can register a host to your box
• Check out PR
• Create a virt-who config for VMware (Ping me for creds if you don't have a login)
• Run the deploy script so it sends the guest/host mapping back
• Register a client to your 6.15/stream box and see if the host/guest mapping works correctly in the legacy UI(if you don't want to spin up a client on vmware, ping me and I can give you a test client to register.)
• Verify that the guest/host mapping looks correct in the legacy UI

Screenshot before:

Screenshot after:

[jeremylenz]

This was changed to display_name recently, in #10538, which is related to theforeman/foreman#9613. I want to make sure we fully understand what caused this, because it's kinda weird to be changing it back so soon..

[jeremylenz]

Marking requested changes so it's not merged accidentally

let's double-check it

[wbclark]

This was changed to display_name recently, in #10538, which is related to theforeman/foreman#9613. I want to make sure we fully understand what caused this, because it's kinda weird to be changing it back so soon..

Hmmm, all the other changes in that PR were from host.name to host.display_name

I wonder if virtual_host.name -> virtual_host.display_name got caught up accidentally in some find and replace?

[chris1984]

This was changed to display_name recently, in #10538, which is related to theforeman/foreman#9613. I want to make sure we fully understand what caused this, because it's kinda weird to be changing it back so soon..

Hmmm, all the other changes in that PR were from host.name to host.display_name

I wonder if virtual_host.name -> virtual_host.display_name got caught up accidentally in some find and replace?

That is what I think happened, I am going to try using short names and see if it makes a difference on the host, if it does not affect the hostname then I think it's fine to switch it, since it's only the old UI.

[jeremylenz]

In Foreman, app/views/api/v2/hosts/base.json.rabl:

node :display_name do |host|
  host.to_label
end

This means that when looking at a Host::Managed API response, it will have a display_name node.

In Katello, app/models/katello/host/subscription_facet.rb:

      belongs_to :hypervisor_host, :class_name => "::Host::Managed"

In app/views/katello/api/v2/subscription_facet/show.json.rabl:

child :hypervisor_host => :virtual_host do
    attributes :id, :name
  end

This means that in the subscription_facet_attributes API response node, the virtual_host will be displayed only with id and name. I think this explains the discrepancy.

We could make this work with display_name as well for consistency by altering that rabl. Or we could leave this as-is. Thoughts?

[wbclark]

We could make this work with display_name as well for consistency by altering that rabl. Or we could leave this as-is. Thoughts?

Thanks @jeremylenz . Could you help me better understand what is the upside vs. effort required for doing so? Is this issue going to come up a different way in the future once the old UI goes away?

[jeremylenz]

@wbclark If we leave it as name, that would mean this field doesn't respect the "Display FQDN for hosts" setting. We should test that. (I don't think it will affect the new UI.)

[chris1984]

I am +1 for changing the rabl so it works with the Display FQDN for hosts setting. Your thoughts @wbclark ?

[wbclark]

Hmmm, I can understand why some users might prefer to see short names only when looking at all hosts, but maybe there is no reason to care when they are looking up the hypervisor a host is mapped to? Not sure tbh.

If the effort is small and you both want to do it, then IMO go for it.

[chris1984]

@jeremylenz updated, here is what the object looks like now:

It works great both in the old and new UI

[jeremylenz]

awesome! And just double-checking, does it display differently in the web UI based on that setting?

[chris1984]

awesome! And just double-checking, does it display differently in the web UI based on that setting?

It looks the same as how it used to be before I made any changes, and we broke the old UI:

Old UI:

New UI:

[chris1984]

Yep, it honors the new setting in the old UI :)

[chris1984]

[test katello]

[jeremylenz]

Nice work! Thanks @chris1984

APJ 👍 from me

[jeremylenz]

[test katello]