Fixes #39167 - set category when REX feature is present#1024
Fixes #39167 - set category when REX feature is present#1024Lukshio merged 3 commits intotheforeman:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Job Wizard prefill behavior so that when a Remote Execution (REX) feature (and its job template) is provided via the URL, the wizard also resolves and preselects the correct job category (e.g., Katello instead of Ansible Commands). It also reduces unnecessary rerenders by stabilizing selector return references and memoizing the Category/Template step container.
Changes:
- Memoize the Category/Template connected step component to reduce rerenders.
- Stabilize several JobWizard selectors via
reselectand stable empty fallbacks. - Update JobWizard defaults application to update
categoryfrom the fetched job template’sjob_category, enabling correct REX feature prefill behavior. - Adjust JobWizard test fixtures/mocks to support additional template IDs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| webpack/JobWizard/steps/CategoryAndTemplate/index.js | Memoizes the connected Category/Template step export to reduce rerenders. |
| webpack/JobWizard/tests/fixtures.js | Expands fixture data and mock API responses to cover multiple templates/categories. |
| webpack/JobWizard/JobWizardSelectors.js | Adds selector memoization and stable fallbacks to reduce referential churn in useSelector. |
| webpack/JobWizard/JobWizard.js | Updates wizard defaults application so fetched template data can drive category selection (REX feature URL prefill). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (categoryRef.current !== job_category) { | ||
| setCategory(job_category); | ||
| } |
There was a problem hiding this comment.
The new logic that updates the wizard category based on the fetched job template’s job_category (important for REX feature URL prefill) doesn’t appear to have a regression test. There are already URL-autofill tests for feature=...; please add an assertion covering that the Category step preselects the expected category when the feature resolves a job template in a different category (e.g., Katello vs Ansible Commands).
| export const selectRouterSearch = state => { | ||
| const { search } = selectRouterLocation(state); | ||
| const { search } = selectRouterLocation(state) || {}; | ||
| return URI.parseQuery(search); |
There was a problem hiding this comment.
selectRouterSearch calls URI.parseQuery with search which can be undefined when router location is missing/empty; URI.js may interpret that as the literal string "undefined" and produce an unexpected query object. Consider defaulting search to an empty string before parsing (and/or returning an empty object when search is falsy).
| return URI.parseQuery(search); | |
| return URI.parseQuery(search || ''); |
| @@ -202,7 +218,7 @@ export const mockApi = api => { | |||
| action.url.search() === | |||
| '?search=job_category%3D%22Puppet%22&per_page=all' | |||
| ? [pupptetJobTemplate] | |||
| : [jobTemplate], | |||
| : wizardJobTemplatesMockList, | |||
| }, | |||
| }); | |||
There was a problem hiding this comment.
In the JOB_TEMPLATES mock, non-Puppet category searches return wizardJobTemplatesMockList, which currently includes a Puppet template. That diverges from the real API behavior (results should be filtered by the requested job_category) and can let category/template selection regressions slip through tests. Consider filtering wizardJobTemplatesMockList based on the search query param (or returning an Ansible-only list for non-Puppet searches).
Lukshio
left a comment
Lukshio
left a comment
There was a problem hiding this comment.
I tried version without reselect and it works well, so I would like to not include another lib
|
Changes looks, good, thanks @MariaAga |
Job wizard does not pre-fill REX feature category and job template when they are in the url
Added also some memo to selectors since while debugging this issues I saw that the component re-renders every few seconds for no reason. selectors stabilization was AI assisted.
The
setCategorywas removed #1016 , but should have instead been updated to a ref to allow REX feature pre-fill.I tested this PR on rex feature usage (Doesnt have to be errata, can also be install package -customized Remote Execution, or other REX features that are customized). Tested on rerun, and on a normal new job.
Description of problem:
In the Manage Errata tab, when applying errata to a specific host using customized Remote Execution, the job category is not being correctly resolved as Katello, but displayed as Ansible Commands.
Steps to Reproduce:
Actual behavior:
When applying errata to a specific host using customized Remote Execution, the job category is incorrectly resolved as Ansible Command.
Expected behavior:
The job category should be displayed as Katello instead of Ansible Command when applying errata.