From e9d84690f91386ce650cea306cabf100cdd640c5 Mon Sep 17 00:00:00 2001 From: Jimmy Conner Date: Mon, 31 Aug 2026 09:32:22 -0500 Subject: [PATCH 1/3] Fix issue with sliced node names in workflow menu --- .../WorkflowOutputNavigation.js | 19 ++++-- .../WorkflowOutputNavigation.test.js | 60 ++++++++++++++++++- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js index 55d29e40..9c297b76 100644 --- a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js +++ b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js @@ -71,10 +71,21 @@ function WorkflowOutputNavigation({ relatedJobs, parentRef }) { setFilterBy((current) => (current === value ? undefined : value)); }; - const nodeLabel = (node) => - stringIsUUID(node.identifier) - ? node.summary_fields.job.name - : node.identifier; + const nodeLabel = (node) => { + if (stringIsUUID(node.identifier)) { + return node.summary_fields.job.name; + } + if (node.identifier) { + return node.identifier; + } + // Sliced-job and federated-inventory workflows create their nodes directly + // rather than copying them from a template node, so identifier is blank, + // and every slice's job carries the same name as the template. Label these + // by position, in the words the toggle uses for the job on screen. + // eslint-disable-next-line no-shadow -- reusing the name reuses the msgid + const currentPosition = jobNodes.indexOf(node) + 1; + return t`Workflow Job ${currentPosition}/${total}`; + }; // Derived rather than held in state: the previous version seeded a useState // from the first render's list, so after navigating within the workflow the diff --git a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.test.js b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.test.js index 4ea48e0b..edd2b595 100644 --- a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.test.js +++ b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.test.js @@ -51,7 +51,29 @@ const relatedJobs = [ }, ]; -function renderAt(jobId) { +// sliced jobs (and federated inventories) build their workflow nodes directly +// rather than from a template node: identifier comes back blank, and every +// slice's job wears the template's name +const slicedJobs = [ + { + id: 30, + job: 301, + identifier: '', + summary_fields: { + job: { id: 301, name: 'Sliced JT', type: 'job', status: 'successful' }, + }, + }, + { + id: 31, + job: 302, + identifier: '', + summary_fields: { + job: { id: 302, name: 'Sliced JT', type: 'job', status: 'failed' }, + }, + }, +]; + +function renderAt(jobId, jobs = relatedJobs) { const history = createMemoryHistory({ initialEntries: [`/jobs/playbook/${jobId}/output`], }); @@ -61,7 +83,7 @@ function renderAt(jobId) { + } /> , @@ -243,4 +265,38 @@ describe('', () => { // no route for that type, so it stays put rather than going to /jobs/undefined/199 expect(history.location.pathname).toBe('/jobs/playbook/101/output'); }); + + test('labels nodes with a blank identifier by their position', async () => { + const { user } = renderAt(301, slicedJobs); + await user.click(screen.getByRole('button')); + // the entry for the job on screen reads the same as the toggle, so it + // appears twice; the other slice appears once, in the menu + await waitFor(() => + expect(screen.getAllByText('Workflow Job 1/2')).toHaveLength(2) + ); + expect(screen.getByText('Workflow Job 2/2')).toBeInTheDocument(); + }); + + test('navigates to a slice picked by its position', async () => { + const { user, history } = renderAt(301, slicedJobs); + await user.click(screen.getByRole('button')); + await waitFor(() => screen.getByText('Workflow Job 2/2')); + await user.click(screen.getByText('Workflow Job 2/2')); + await waitFor(() => + expect(history.location.pathname).toBe('/jobs/playbook/302/output') + ); + }); + + test('keeps positional labels stable under a status filter', async () => { + const { user } = renderAt(301, slicedJobs); + await user.click(screen.getByRole('button')); + await user.click(screen.getByRole('option', { name: /Failed/ })); + // only the failed slice survives, still wearing its original position + await waitFor(() => + expect(screen.getByText('Workflow Job 2/2')).toBeInTheDocument() + ); + expect( + screen.queryByRole('option', { name: 'Workflow Job 1/2' }) + ).not.toBeInTheDocument(); + }); }); From 35b598e0a6c4883b48ec89f7a7407c8094bc705e Mon Sep 17 00:00:00 2001 From: Jimmy Conner Date: Mon, 31 Aug 2026 10:35:46 -0500 Subject: [PATCH 2/3] Small improvement --- .../WorkflowOutputNavigation.js | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js index 9c297b76..61d12374 100644 --- a/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js +++ b/awx/ui/src/components/WorkflowOutputNavigation/WorkflowOutputNavigation.js @@ -56,12 +56,16 @@ function WorkflowOutputNavigation({ relatedJobs, parentRef }) { ); // 1-based, and 0 when the job on screen is not one of the workflow's nodes - const currentPosition = + const viewedPosition = jobNodes.findIndex(({ job: jobId }) => `${jobId}` === id) + 1; - // named so the extracted message reads {currentPosition}/{total} rather than - // leaving translators with a positional {0} const total = jobNodes.length; + // the parameter and total are named so the extracted message reads + // {currentPosition}/{total} rather than leaving translators with a + // positional {0} + const positionLabel = (currentPosition) => + t`Workflow Job ${currentPosition}/${total}`; + const statusLabels = { Failed: t`Failed`, Successful: t`Successful`, @@ -82,9 +86,9 @@ function WorkflowOutputNavigation({ relatedJobs, parentRef }) { // rather than copying them from a template node, so identifier is blank, // and every slice's job carries the same name as the template. Label these // by position, in the words the toggle uses for the job on screen. - // eslint-disable-next-line no-shadow -- reusing the name reuses the msgid - const currentPosition = jobNodes.indexOf(node) + 1; - return t`Workflow Job ${currentPosition}/${total}`; + return positionLabel( + jobNodes.findIndex((candidate) => candidate.id === node.id) + 1 + ); }; // Derived rather than held in state: the previous version seeded a useState @@ -154,8 +158,8 @@ function WorkflowOutputNavigation({ relatedJobs, parentRef }) { )} {!filterBy && - (currentPosition > 0 - ? t`Workflow Job ${currentPosition}/${total}` + (viewedPosition > 0 + ? positionLabel(viewedPosition) : t`Workflow Jobs (${total})`)} )} From 14e9400fac4819c478d48d2f849131c5c127d624 Mon Sep 17 00:00:00 2001 From: Jimmy Conner Date: Mon, 31 Aug 2026 10:50:34 -0500 Subject: [PATCH 3/3] Menu is currently limited to 25 items because of the page size --- awx/ui/src/screens/Job/Job.js | 5 ++++- awx/ui/src/screens/Job/Job.test.js | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/awx/ui/src/screens/Job/Job.js b/awx/ui/src/screens/Job/Job.js index 5ee2fd09..b01a980c 100644 --- a/awx/ui/src/screens/Job/Job.js +++ b/awx/ui/src/screens/Job/Job.js @@ -65,7 +65,10 @@ function Job({ setBreadcrumb }) { const { data: { results }, } = await getJobModel('workflow_job').readNodes( - jobDetailData.summary_fields.source_workflow_job.id + jobDetailData.summary_fields.source_workflow_job.id, + // without this the API returns its default page of 25, which + // truncates the workflow navigation menu; 200 is MAX_PAGE_SIZE + { page_size: 200 } ); relatedJobData = results; } diff --git a/awx/ui/src/screens/Job/Job.test.js b/awx/ui/src/screens/Job/Job.test.js index 2f668274..95e75f85 100644 --- a/awx/ui/src/screens/Job/Job.test.js +++ b/awx/ui/src/screens/Job/Job.test.js @@ -1,5 +1,6 @@ import React from 'react'; -import { waitForElementToBeRemoved } from '@testing-library/react'; +import { waitFor, waitForElementToBeRemoved } from '@testing-library/react'; +import { ProjectUpdatesAPI, WorkflowJobsAPI } from 'api'; import { renderWithContexts } from '../../../testUtils/rtlContexts'; import Job from './Job'; @@ -25,4 +26,27 @@ describe('', () => { container.querySelector('[role="progressbar"]') ); }); + + test('requests a full page of workflow nodes for the navigation menu', async () => { + ProjectUpdatesAPI.readDetail.mockResolvedValue({ + data: { + id: 1, + type: 'project_update', + related: { source_workflow_job: '/api/v2/workflow_jobs/99/' }, + summary_fields: { source_workflow_job: { id: 99 } }, + }, + }); + ProjectUpdatesAPI.readEventOptions.mockResolvedValue({ data: {} }); + WorkflowJobsAPI.readNodes.mockResolvedValue({ data: { results: [] } }); + + renderWithContexts( {}} />); + + // the API's default page is 25 nodes, which truncates the menu for large + // (e.g. heavily sliced) workflows; the fetch must ask for MAX_PAGE_SIZE + await waitFor(() => + expect(WorkflowJobsAPI.readNodes).toHaveBeenCalledWith(99, { + page_size: 200, + }) + ); + }); });