Fixes #38265 - Use jquery data for nested hostgroups #11335
Conversation
| orgIds = [$("#host_organization_id").val()]; | ||
| }; | ||
| orgIds = orgIds.map(id => Number(id)); |
There was a problem hiding this comment.
Copilot and myself came to the conclusion, that this is technically 100% the same but the "arrow" function is the modern one. I would stay with the modern one :)
There was a problem hiding this comment.
I changed it back to ES5-style JavaScript because that's what the rest of the file is, and I got an eslint warning about it.
| if(orgIds === undefined || orgIds === null || orgIds.length === 0) { | ||
| var orgIdsElem = $("#hostgroup_organization_ids"); | ||
| var orgIds = orgIdsElem.val(); | ||
| if (orgIds === undefined || orgIds === null || orgIds.length === 0) { |
There was a problem hiding this comment.
Not tested, but copilot suggest this:
const orgIdsElem = $("#hostgroup_organization_ids");
let orgIds = orgIdsElem.val() ?? orgIdsElem.data('useds') ?? [$("#host_organization_id").val()];
The nullish coalescing operatos looks like exactly what is required here:
The nullish
There was a problem hiding this comment.
Normally I would love to write it this way, but this also is not ES5-compatible, unfortunately.
There was a problem hiding this comment.
@MariaAga Now that I think of this.. Does jQuery 3 mean we can start using modern JS in every place besides Angular pages?
There was a problem hiding this comment.
I'm actually not sure what prevented us before from using es5/6?
|
Pretty funny that you are working on the same day/same hour on something similar than myself in #11334 |
|
Tested. It worked well. Before
After
|
(cherry picked from commit bf83a7a)
(cherry picked from commit bf83a7a)
What are the changes introduced in this pull request?
In the host_and_hostgroup_edit form, there is a hidden select element created with the ID
#hostgroup_organization_ids.Its value is an array of organization IDs. When a user changes the content source using the dropdown, this hidden element is used to get organization IDs to query. This is done in order to populate the Lifecycle Environment and Content View fields.
Previously (I'm assuming before jQuery 3?) this was accessed via
Problem is, when creating a nested hostgroup, that jQuery call seems to return
[]-- even though the vanilla JS calldocument.querySelector("#hostgroup_organization_ids")confirms the element is there, and has the required data ([1]in my case).I fixed the issue by using
.data('useds')instead of.val(). This fixed the nested hostgroup issue, but broke hostgroup edit.So I altered the logic to only use
.data('useds')in the specific case of nested hostgroups.Considerations taken when implementing this change?
I am not sure why my solution works, or why it breaks the other flow, or if my solution is at all correct.
What are the testing steps for this pull request?
Steps to Reproduce:
Set up a Foreman server with a Smart Proxy with Content and have multiple LCE
Create a hostgroup HG1 with Foreman server as content source and choose any set of LCE and CV
Now create a nested hostgroup HG2 from HG1
and
Change the content source to Smart Proxy
Now try to select other cv or lce
Before patch:
It wont populate any and says "No matches found"
Expected behavior:
It should populate other cv and lce