From 9aea9b44a79d332b7a23bc2bcfa757703643f721 Mon Sep 17 00:00:00 2001 From: blaipr Date: Sun, 14 Jun 2026 05:44:44 +0200 Subject: [PATCH 1/4] Convert Credential route tree to react-router v6 Routes Migrate the Credential list and detail route trees from the react-router v5 // API to v6 / via react-router-dom-v5-compat, following the pattern used for the earlier screen conversions. - Credentials.js: -> ; child routes use the element prop; the detail route uses /credentials/:id/* so the nested tree matches the rest of the path. - Credential.js: -> with relative child paths (details, edit, access, job_templates); the exact becomes an index route that s to details; the duplicated v5 catch-all not-found routes collapse to a single path="*" route, and the useRouteMatch({path}) match.params.id is replaced with the id route param from useParams. - Rewrite both test suites with renderWithContexts (RTL) to assert real v6 route resolution, including the kind-gated Job Templates tab, the index redirect, the unknown sub-route not-found error, and the 404 detail error. --- awx/ui/src/screens/Credential/Credential.js | 73 +++---- .../src/screens/Credential/Credential.test.js | 184 ++++++++++-------- awx/ui/src/screens/Credential/Credentials.js | 36 ++-- .../screens/Credential/Credentials.test.js | 59 +++++- 4 files changed, 205 insertions(+), 147 deletions(-) diff --git a/awx/ui/src/screens/Credential/Credential.js b/awx/ui/src/screens/Credential/Credential.js index ec9cf3ed2..1de3ea226 100644 --- a/awx/ui/src/screens/Credential/Credential.js +++ b/awx/ui/src/screens/Credential/Credential.js @@ -3,15 +3,14 @@ import { useLingui } from '@lingui/react/macro'; import { CaretLeftIcon } from '@patternfly/react-icons'; import { Card, PageSection } from '@patternfly/react-core'; +import { Link } from 'react-router-dom'; import { - Switch, + Routes, + Route, + Navigate, useParams, useLocation, - useRouteMatch, - Route, - Redirect, - Link, -} from 'react-router-dom'; +} from 'react-router-dom-v5-compat'; import useRequest from 'hooks/useRequest'; import { ResourceAccessList } from 'components/ResourceAccessList'; import ContentError from 'components/ContentError'; @@ -40,10 +39,6 @@ const unacceptableCredentialTypes = [ function Credential({ setBreadcrumb }) { const { t } = useLingui(); const { pathname } = useLocation(); - - const match = useRouteMatch({ - path: '/credentials/:id', - }); const { id } = useParams(); const { @@ -135,45 +130,37 @@ function Credential({ setBreadcrumb }) { {showCardHeader && } {!hasContentLoading && credential && ( - - + } /> + } + /> + } /> - {credential && [ - - - , - - - , - + - , - + } + /> + - , - - {!hasContentLoading && ( - - {match.params.id && ( - - {t`View Credential Details`} - - )} - - )} - , - ]} - - {!hasContentLoading && ( + } + /> + {id && ( @@ -181,9 +168,9 @@ function Credential({ setBreadcrumb }) { )} - )} - - + } + /> + )} diff --git a/awx/ui/src/screens/Credential/Credential.test.js b/awx/ui/src/screens/Credential/Credential.test.js index 152d86dc6..552bb6a4e 100644 --- a/awx/ui/src/screens/Credential/Credential.test.js +++ b/awx/ui/src/screens/Credential/Credential.test.js @@ -1,107 +1,129 @@ import React from 'react'; -import { act } from 'react-dom/test-utils'; +import { screen, waitFor } from '@testing-library/react'; import { createMemoryHistory } from 'history'; +import { Routes, Route } from 'react-router-dom-v5-compat'; import { CredentialsAPI } from 'api'; -import { - mountWithContexts, - waitForElement, -} from '../../../testUtils/enzymeHelpers'; +import { renderWithContexts } from '../../../testUtils/rtlContexts'; import mockMachineCredential from './shared/data.machineCredential.json'; -import mockSCMCredential from './shared/data.scmCredential.json'; import mockCyberArkCredential from './shared/data.cyberArkCredential.json'; import Credential from './Credential'; -jest.mock('../../api'); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), - useRouteMatch: () => ({ - url: '/credentials/2', - params: { id: 2 }, - }), -})); +jest.mock('../../api/models/Credentials'); + +// Markers for the routed tab panels, so assertions are about which branch of +// the nested v6 tree resolves. +jest.mock('./CredentialDetail', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'CredentialDetail'), + }; +}); +jest.mock('./CredentialEdit', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'CredentialEdit'), + }; +}); +jest.mock('components/RelatedTemplateList', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'RelatedTemplateList'), + }; +}); +jest.mock('components/ResourceAccessList', () => { + const ReactLib = require('react'); + return { + ResourceAccessList: () => + ReactLib.createElement('div', null, 'ResourceAccessList'), + }; +}); + +// Credential uses paths relative to its parent route, so mount it under the +// same /credentials/:id/* route that Credentials.js gives it in the app. +function renderAt(path) { + const history = createMemoryHistory({ initialEntries: [path] }); + return renderWithContexts( + + {}} />} + /> + , + { context: { router: { history } } } + ); +} describe('', () => { - let wrapper; + beforeEach(() => { + CredentialsAPI.readDetail.mockResolvedValue({ data: mockMachineCredential }); + }); + afterEach(() => { jest.clearAllMocks(); + }); - wrapper.unmount(); + test('fetches the credential detail', async () => { + renderAt('/credentials/2/details'); + expect(await screen.findByText('CredentialDetail')).toBeInTheDocument(); + // real route params are strings (the old enzyme test mocked a number) + expect(CredentialsAPI.readDetail).toHaveBeenCalledWith('2'); }); - test('initially renders user-based machine credential successfully', async () => { - CredentialsAPI.readDetail.mockResolvedValueOnce({ - data: mockMachineCredential, - }); - await act(async () => { - wrapper = mountWithContexts( {}} />); - }); - wrapper.update(); - expect(wrapper.find('Credential').length).toBe(1); - expect(wrapper.find('RoutedTabs li').length).toBe(4); + test('renders the edit panel at /edit', async () => { + renderAt('/credentials/2/edit'); + expect(await screen.findByText('CredentialEdit')).toBeInTheDocument(); }); - test('initially renders user-based SCM credential successfully', async () => { - CredentialsAPI.readDetail.mockResolvedValueOnce({ - data: mockSCMCredential, - }); - await act(async () => { - wrapper = mountWithContexts( {}} />); - }); - wrapper.update(); - expect(wrapper.find('Credential').length).toBe(1); - expect(wrapper.find('RoutedTabs li').length).toBe(3); + test('renders the access panel at /access', async () => { + renderAt('/credentials/2/access'); + expect(await screen.findByText('ResourceAccessList')).toBeInTheDocument(); }); - test('should render expected tabs', async () => { - const expectedTabs = [ - 'Back to Credentials', - 'Details', - 'Access', - 'Job Templates', - ]; - await act(async () => { - wrapper = mountWithContexts( {}} />); - }); - wrapper.find('RoutedTabs li').forEach((tab, index) => { - expect(tab.text()).toEqual(expectedTabs[index]); - }); + test('renders the job templates panel at /job_templates', async () => { + renderAt('/credentials/2/job_templates'); + expect(await screen.findByText('RelatedTemplateList')).toBeInTheDocument(); + }); + + test('redirects the index path to details', async () => { + const { history } = renderAt('/credentials/2'); + expect(await screen.findByText('CredentialDetail')).toBeInTheDocument(); + await waitFor(() => + expect(history.location.pathname).toBe('/credentials/2/details') + ); + }); + + test('shows the Job Templates tab for an acceptable credential kind', async () => { + renderAt('/credentials/2/details'); + expect(await screen.findByText('CredentialDetail')).toBeInTheDocument(); + expect(screen.getByText('Job Templates')).toBeInTheDocument(); }); - test('should not render job template tab', async () => { - CredentialsAPI.readDetail.mockResolvedValueOnce({ + test('hides the Job Templates tab for a registry credential', async () => { + CredentialsAPI.readDetail.mockResolvedValue({ data: { ...mockCyberArkCredential, kind: 'registry' }, }); - const expectedTabs = ['Back to Credentials', 'Details', 'Access']; - await act(async () => { - wrapper = mountWithContexts( {}} />); - }); - wrapper.find('RoutedTabs li').forEach((tab, index) => { - expect(tab.text()).toEqual(expectedTabs[index]); - }); + renderAt('/credentials/2/details'); + expect(await screen.findByText('CredentialDetail')).toBeInTheDocument(); + expect(screen.queryByText('Job Templates')).not.toBeInTheDocument(); }); - test('should show content error when user attempts to navigate to erroneous route', async () => { - const history = createMemoryHistory({ - initialEntries: ['/credentials/2/foobar'], - }); - await act(async () => { - wrapper = mountWithContexts( {}} />, { - context: { - router: { - history, - route: { - location: history.location, - match: { - params: { id: 1 }, - url: '/credentials/2/foobar', - path: '/credentials/2/foobar', - }, - }, - }, - }, - }); - }); - await waitForElement(wrapper, 'ContentError', (el) => el.length === 1); + test('shows a not-found error on an unknown sub-route', async () => { + renderAt('/credentials/2/foobar'); + expect( + await screen.findByText('View Credential Details') + ).toBeInTheDocument(); + expect(screen.queryByText('CredentialDetail')).not.toBeInTheDocument(); + }); + + test('shows a not-found error when the detail request 404s', async () => { + const err = new Error('not found'); + err.response = { status: 404 }; + CredentialsAPI.readDetail.mockRejectedValue(err); + renderAt('/credentials/2/details'); + expect(await screen.findByText('Credential not found.')).toBeInTheDocument(); + expect(screen.queryByText('CredentialDetail')).not.toBeInTheDocument(); }); }); -describe(' should not show job template tab', () => {}); diff --git a/awx/ui/src/screens/Credential/Credentials.js b/awx/ui/src/screens/Credential/Credentials.js index 9fdf175da..58f9a53c3 100644 --- a/awx/ui/src/screens/Credential/Credentials.js +++ b/awx/ui/src/screens/Credential/Credentials.js @@ -1,5 +1,5 @@ import React, { useState, useCallback } from 'react'; -import { Route, Switch } from 'react-router-dom'; +import { Routes, Route } from 'react-router-dom-v5-compat'; import { useLingui } from '@lingui/react/macro'; import { Config } from 'contexts/Config'; @@ -41,19 +41,27 @@ function Credentials() { streamType="credential" breadcrumbConfig={breadcrumbConfig} /> - - - {({ me }) => } - - - - - - - - - - + + {({ me }) => } + } + /> + {/* /* so the nested route tree can match the rest */} + } + /> + + + + } + /> + ); } diff --git a/awx/ui/src/screens/Credential/Credentials.test.js b/awx/ui/src/screens/Credential/Credentials.test.js index 23cf1f871..156041fc7 100644 --- a/awx/ui/src/screens/Credential/Credentials.test.js +++ b/awx/ui/src/screens/Credential/Credentials.test.js @@ -1,16 +1,57 @@ import React from 'react'; -import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; +import { screen } from '@testing-library/react'; +import { createMemoryHistory } from 'history'; +import { renderWithContexts } from '../../../testUtils/rtlContexts'; import Credentials from './Credentials'; +jest.mock('../../api/models/Credentials'); + +// Replace the routed children with markers so the assertions are purely about +// which branch of the v6 tree resolves for a given URL. +jest.mock('./CredentialList', () => { + const ReactLib = require('react'); + return { + __esModule: true, + CredentialList: () => ReactLib.createElement('div', null, 'CredentialList'), + }; +}); +jest.mock('./CredentialAdd', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'CredentialAdd'), + }; +}); +jest.mock('./Credential', () => { + const ReactLib = require('react'); + return { + __esModule: true, + default: () => ReactLib.createElement('div', null, 'Credential detail'), + }; +}); + +function renderAt(path) { + const history = createMemoryHistory({ initialEntries: [path] }); + return renderWithContexts(, { + context: { router: { history } }, + }); +} + describe('', () => { - test('should set breadcrumb config', () => { - const wrapper = mountWithContexts(); + test('renders the list at /credentials', async () => { + renderAt('/credentials'); + expect(await screen.findByText('CredentialList')).toBeInTheDocument(); + }); + + test('renders the add form at /credentials/add', async () => { + renderAt('/credentials/add'); + expect(await screen.findByText('CredentialAdd')).toBeInTheDocument(); + expect(screen.queryByText('CredentialList')).not.toBeInTheDocument(); + }); - const header = wrapper.find('ScreenHeader'); - expect(header.prop('streamType')).toEqual('credential'); - expect(header.prop('breadcrumbConfig')).toEqual({ - '/credentials': 'Credentials', - '/credentials/add': 'Create New Credential', - }); + test('renders the detail subtree at /credentials/:id', async () => { + renderAt('/credentials/2/details'); + expect(await screen.findByText('Credential detail')).toBeInTheDocument(); + expect(screen.queryByText('CredentialList')).not.toBeInTheDocument(); }); }); From acc60bf59a89885fa34f83f78bdb3cb2d5127fb8 Mon Sep 17 00:00:00 2001 From: blaipr Date: Sun, 14 Jun 2026 13:44:31 +0200 Subject: [PATCH 2/4] Fix CredentialEdit useParams under the v6 route tree CredentialEdit read the credential id via v5 useParams(), which returns {} under the v6 route tree (no v5 Route ancestor), leaving the id undefined on update/cancel. Read useParams from react-router-dom-v5-compat instead, and point the test's useParams mock at react-router-dom-v5-compat. --- .../src/screens/Credential/CredentialEdit/CredentialEdit.js | 3 +-- .../Credential/CredentialEdit/CredentialEdit.test.js | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.js b/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.js index adcc31b6f..1279e8c3a 100644 --- a/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.js +++ b/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.js @@ -1,6 +1,5 @@ import React, { useCallback, useEffect, useState } from 'react'; -import { useParams } from 'react-router-dom'; -import { useNavigate } from 'react-router-dom-v5-compat'; +import { useNavigate, useParams } from 'react-router-dom-v5-compat'; import { CardBody } from 'components/Card'; import { CredentialsAPI, diff --git a/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.test.js b/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.test.js index 2ce886564..fc40c242d 100644 --- a/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.test.js +++ b/awx/ui/src/screens/Credential/CredentialEdit/CredentialEdit.test.js @@ -16,8 +16,10 @@ import { import CredentialEdit from './CredentialEdit'; jest.mock('../../../api'); -jest.mock('react-router-dom', () => ({ - ...jest.requireActual('react-router-dom'), +// The component reads useParams from react-router-dom-v5-compat (the route +// tree is v6); mock it there, keeping the rest of the module real. +jest.mock('react-router-dom-v5-compat', () => ({ + ...jest.requireActual('react-router-dom-v5-compat'), useParams: () => ({ id: 3, }), From f9b0e036900408b0049ec07df0eec58cdd400601 Mon Sep 17 00:00:00 2001 From: blaipr Date: Sun, 14 Jun 2026 22:44:39 +0200 Subject: [PATCH 3/4] Address review comment: fix the doubled '/* /*' in the nested-route JSX comment --- awx/ui/src/screens/Credential/Credentials.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/ui/src/screens/Credential/Credentials.js b/awx/ui/src/screens/Credential/Credentials.js index 58f9a53c3..c3fac2185 100644 --- a/awx/ui/src/screens/Credential/Credentials.js +++ b/awx/ui/src/screens/Credential/Credentials.js @@ -48,7 +48,7 @@ function Credentials() { {({ me }) => } } /> - {/* /* so the nested route tree can match the rest */} + {/* so the nested route tree can match the rest */} } From a6824d4acb5aa5923f10201c335ecb2070f6e281 Mon Sep 17 00:00:00 2001 From: blaipr Date: Sun, 14 Jun 2026 23:03:04 +0200 Subject: [PATCH 4/4] Address review comment: restore ScreenHeader breadcrumb coverage The enzyme suite asserted the Credentials ScreenHeader breadcrumb/title mapping; the RTL route-resolution tests had dropped it. Capture the ScreenHeader props and assert the stream type and breadcrumb config so a regression in breadcrumbConfig is caught again. --- .../screens/Credential/Credentials.test.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/awx/ui/src/screens/Credential/Credentials.test.js b/awx/ui/src/screens/Credential/Credentials.test.js index 156041fc7..ee9ca120d 100644 --- a/awx/ui/src/screens/Credential/Credentials.test.js +++ b/awx/ui/src/screens/Credential/Credentials.test.js @@ -6,6 +6,17 @@ import Credentials from './Credentials'; jest.mock('../../api/models/Credentials'); +// Capture the ScreenHeader props so we can assert the breadcrumb/title +// mapping that the previous enzyme suite covered. +let mockScreenHeaderProps; +jest.mock('components/ScreenHeader', () => ({ + __esModule: true, + default: (props) => { + mockScreenHeaderProps = props; + return null; + }, +})); + // Replace the routed children with markers so the assertions are purely about // which branch of the v6 tree resolves for a given URL. jest.mock('./CredentialList', () => { @@ -54,4 +65,15 @@ describe('', () => { expect(await screen.findByText('Credential detail')).toBeInTheDocument(); expect(screen.queryByText('CredentialList')).not.toBeInTheDocument(); }); + + test('sets the ScreenHeader stream type and breadcrumb config', async () => { + renderAt('/credentials'); + expect(await screen.findByText('CredentialList')).toBeInTheDocument(); + expect(mockScreenHeaderProps).toBeDefined(); + expect(mockScreenHeaderProps.streamType).toBe('credential'); + expect(mockScreenHeaderProps.breadcrumbConfig).toEqual({ + '/credentials': 'Credentials', + '/credentials/add': 'Create New Credential', + }); + }); });