Fixes #35762 - No host error after editing host's interfaces #9564
Conversation
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Issues: #35762 |
theforeman-bot
added
Waiting on contributor
Not yet reviewed
Legacy JS
There was a problem hiding this comment.
Thanks, @kmalyjur!
I think the proper solution with these short/full names could be achieved after #9613 is merged and we agree on some united logic about the names for managed/unmanaged hosts.
But as for now, (a) simple workaround(s) will do. One suggestion inline makes the change smaller, but seems to fix the issue as well. It freaks me out I didn't notice it previously. Suggest to do more testing :/
UPD: Could you squash/remove commits related to rebasing? It breaks one of the checks for the bot.
app/helpers/hosts_helper.rb
Outdated
| @@ -247,7 +247,7 @@ def reports_show | |||
|
|
|||
| def name_field(host) | |||
| return if host.name.blank? | |||
| host.managed? ? host.shortname : host.name | |||
There was a problem hiding this comment.
I don't think we need to touch this. Simple host_name_el.data('managed') === false removal will do.
There was a problem hiding this comment.
Thank you for your review!
You are right - this removal was not necessary. However, I want to let you know I made the change so that when editing the host, the name of the managed and unmanaged host is just the shortname - now it is shortname + domain when the host is unmanaged. But also, I am not sure in which form it should be, and it might be resolved after #9613 that you mentioned.
| @@ -391,9 +391,7 @@ $(document).on('change', '.virtual', function() { | |||
| function construct_host_name() { | |||
| var host_name_el = $('#host_name') | |||
| var host_name = host_name_el.val(); | |||
| if (host_name_el.data('appendDomainNameForHosts') === false || | |||
| host_name_el.data('managed') === false | |||
There was a problem hiding this comment.
It would be better to remove https://github.com/ofedoren/foreman/blob/develop/app/views/hosts/_form.html.erb#L36 as well.
|
@ofedoren is this pr waiting for some pr or changes? |
|
@MariaAga, this wait for me :) Currently re-checking this out. |
ofedoren
added
Needs re-review
Needs cherrypick
|
I'll re-run the tests just in case... ok to test |
|
Late to the party, but there's actually a much more reliable fix: you can get the I suppose the only edge case is when users opted out of the new detail page, but I doubt we need to cater to that. Another solution is to use the ID instead of the FQDN since it's stable. |
|
Hi @ekohl, thank you for your review. However, I don't get the part with the Location. |
When editing the interfaces of an unmanaged host, this error occurred:
The issue was caused by the unmanaged host's primary interface name being shortname+domain; meanwhile, the same name of a managed host is just shortname.
This fix follows up the 3bc478f.