Fixes #36160 - Redefine append domain names setting

[Dyrkon]

This PR aims to unify the format of host names stored in the database and the way they are displayed. With this change, the name of the host is always going to be stored with the domain name appended.

The setting formerly named append_domain_name_for_hosts is now renamed to display_fqdn_for_hosts because it will only impact how the names are displayed from now. This means dashboards and breadcrumbs are going to display the whole FQDN if you choose to.

[theforeman-bot]

Can one of the admins verify this patch?

[theforeman-bot]

Can one of the admins verify this patch?

[theforeman-bot]

Can one of the admins verify this patch?

[ares]

There are more places that now needs to stop using the setting, e.g.

foreman/app/services/name_synchronizer.rb

Line 28 in 3347fa4

Setting[:append_domain_name_for_hosts] ? @interface.name : @interface.shortname

or the host form, right?

[stejskalleos]

Few other places to update:

• app/views/hosts/_form.html.erb
• app/services/name_synchronizer.rb
• test/unit/name_synchronizer_test.rb
• test/models/lookup_value_test.rb

and rubocop

[ofedoren]

What about this setting?

I guess this PR is going to force FQDN to always be stored as a host name, isn't it? If so, does this affect only managed hosts? What about unmanaged or virtual?

Also, do I understand right that from now on the setting should change only the way how to display name of a host? Like in "display short or FQDN" manner with always saving fqdn? I'd also vote to renaming the setting or at least mention that this affect only the view in the description.

Related to the problem: currently I'm kinda mad due to upcoming patches and fixes for UI (#9047 and #9140 and #9201 and #9564 and #9591) to fix almost the same thing again and again. Could we maybe unify the logic and the place where we display hostname? Currently I found few places (https://github.com/theforeman/foreman/blob/develop/app/helpers/hosts_helper.rb#L250 https://github.com/theforeman/foreman/blob/develop/app/models/concerns/orchestration/compute.rb#L47 https://github.com/theforeman/foreman/blob/develop/app/services/name_synchronizer.rb#L28 https://github.com/theforeman/foreman/blob/develop/app/assets/javascripts/host_edit_interfaces.js#L71

foreman/app/assets/javascripts/host_edit_interfaces.js

Line 402 in fc0ad65

return domain_name ? fqdn(host_name, domain_name) : host_name;

) where we deal with host names. Is it me being naive or we really do have mess in it?

[Dyrkon]

What about this setting?

I think that this setting focuses on a different area than the one in this PR. I don't think it is related or should be changed.

I guess this PR is going to force FQDN to always be stored as a host name, isn't it? If so, does this affect only managed hosts? What about unmanaged or virtual?

Yes, this PR proposes always storing the FQDN. I don't know what do you mean by

What about unmanaged or virtual?

The change in model is in base.rb, so it will affect unmanaged as well. It shouldn't have an effect on virtual hosts according to @stejskalleos.

Also, do I understand right that from now on the setting should change only the way how to display name of a host? Like in "display short or FQDN" manner with always saving fqdn? I'd also vote to renaming the setting or at least mention that this affect only the view in the description.

Yes, you understand correctly that the setting will only control the view and I already changed the setting description.

Related to the problem: currently I'm kinda mad due to upcoming patches and fixes for UI (#9047 and #9140 and #9201 and #9564 and #9591) to fix almost the same thing again and again. Could we maybe unify the logic and the place where we display hostname? Currently I found few places (https://github.com/theforeman/foreman/blob/develop/app/helpers/hosts_helper.rb#L250 https://github.com/theforeman/foreman/blob/develop/app/models/concerns/orchestration/compute.rb#L47 https://github.com/theforeman/foreman/blob/develop/app/services/name_synchronizer.rb#L28 https://github.com/theforeman/foreman/blob/develop/app/assets/javascripts/host_edit_interfaces.js#L71

foreman/app/assets/javascripts/host_edit_interfaces.js

Line 402 in fc0ad65

return domain_name ? fqdn(host_name, domain_name) : host_name;

) where we deal with host names. Is it me being naive or we really do have mess in it?

Wouldn't this simplify things by having FQDN at all times and control only the way it is displayed? @ofedoren

[Dyrkon]

@Lennonka

[Lennonka]

Suggestion for UI Description microcopy.

[Lennonka]

Suggested change

	description: N_('Foreman will display domain names appended to hostnames.'),
	description: N_('Display names of hosts as FQDNs. If disabled, only display names of hosts as hostnames.'),

Not sure about the terminology here. If "shortname" is a thing, then maybe say "shortnames" instead of "hostnames". But to my understanding it's called hostnames and I'd prefer the common industry term.

Edit: Also I'd remove "Foreman" from the description, because that wouldn't work well with downstream branding.

[Dyrkon]

You are spot on. I think someone came up with when naming the variable and it caught on.

[stejskalleos]

JS tests are failing:

Error:   81:5  error  ouiaId property is missing in PatternFly component 'Card'   @theforeman/rules/require-ouiaid
Error:   89:9  error  ouiaId property is missing in PatternFly component 'Modal'  @theforeman/rules/require-ouiaid

Can you add the ouiaid field? It's for our automation framework (I think), check with QAs what the value of the fields should be.

[MariaAga]

js Test failures are not related to this pr, I'll open a pr to fix them

[stejskalleos]

ok to test

[stejskalleos]

🍏 LGTM

Works as expected, let's get this in.

[stejskalleos]

Thanks @Dyrkon and others!

[Dyrkon]

Big thanks for everyone who commented and helped with the PR!