From c5cc85e547db95e7a04cffc93c5724aa6994ecfe Mon Sep 17 00:00:00 2001 From: fxiang1 Date: Tue, 28 Apr 2026 14:08:19 -0400 Subject: [PATCH 1/3] Remove placement feature gate Signed-off-by: fxiang1 --- backend/config/enhancedPlacement | 1 - frontend/public/locales/en/translation.json | 8 +- frontend/src/atoms.ts | 2 +- .../AdvancedConfiguration.test.tsx | 31 +- .../Applications/AdvancedConfiguration.tsx | 144 ++------- .../ApplicationDetails.test.tsx | 11 +- .../ApplicationDetails/ApplicationDetails.tsx | 15 +- .../components/ToggleSelector.tsx | 10 +- .../PolicyDetailsOverview.test.tsx | 48 --- .../policy-details/PolicyDetailsOverview.tsx | 296 ++---------------- .../components/PolicySetCard.test.tsx | 4 +- .../policy-sets/components/PolicySetCard.tsx | 5 +- .../Infrastructure/Clusters/ClustersPage.tsx | 15 +- .../ClusterOverview/ClusterOverview.tsx | 5 +- .../CreatePlacement/EditPlacement.tsx | 10 +- .../CreatePlacement/PlacementWizard.tsx | 6 +- .../PlacementDetails.test.tsx | 3 +- .../Clusters/Placements/Placements.test.tsx | 3 +- frontend/src/wizards/Argo/ArgoWizard.tsx | 6 +- frontend/src/wizards/Placement/Placement.tsx | 2 +- .../Placement/PlacementSection.test.tsx | 16 +- .../wizards/Placement/PlacementSection.tsx | 160 +++++----- .../Placement/usePlacementDebug.test.ts | 28 +- .../wizards/Placement/usePlacementDebug.ts | 8 +- 24 files changed, 182 insertions(+), 655 deletions(-) delete mode 100644 backend/config/enhancedPlacement diff --git a/backend/config/enhancedPlacement b/backend/config/enhancedPlacement deleted file mode 100644 index 26ed6c9bbe3..00000000000 --- a/backend/config/enhancedPlacement +++ /dev/null @@ -1 +0,0 @@ -enabled \ No newline at end of file diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index e060891fb27..4ebcdc2856f 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -1,7 +1,4 @@ { - " No status: ": " No status: ", - " No violations: ": " No violations: ", - " Violations: ": " Violations: ", "\"{{name}}\" is not an existing Ansible job": "\"{{name}}\" is not an existing Ansible job", "\"{{name}}\" is not an existing Ansible workflow": "\"{{name}}\" is not an existing Ansible workflow", "\"{{testSecret}}\" is not an existing Ansible credential": "\"{{testSecret}}\" is not an existing Ansible credential", @@ -1312,7 +1309,6 @@ "Displays the namespace of the application resource, which by default is where the application deploys other resources. For Argo applications, this is the destination namespace.": "Displays the namespace of the application resource, which by default is where the application deploys other resources. For Argo applications, this is the destination namespace.", "Displays the number of applications using the subscription. Click to search for all related applications.": "Displays the number of applications using the subscription. Click to search for all related applications.", "Displays the number of local subscriptions using the channel. Click to search for all related subscriptions.": "Displays the number of local subscriptions using the channel. Click to search for all related subscriptions.", - "Displays the number of remote and local clusters where resources are deployed because of the placement.": "Displays the number of remote and local clusters where resources are deployed because of the placement.", "Displays the number of remote and local clusters where resources for the subscription are deployed. Click to search for all related clusters.": "Displays the number of remote and local clusters where resources for the subscription are deployed. Click to search for all related clusters.", "Displays the number of remote and local clusters where resources from the channel are deployed.": "Displays the number of remote and local clusters where resources from the channel are deployed.", "Displays the type of the application. ": "Displays the type of the application. ", @@ -2167,7 +2163,7 @@ "No permissions found": "No permissions found", "No placement": "No placement", "No placement decisions": "No placement decisions", - "No placement selectors found": "No placement selectors found", + "No placements found": "No placements found", "No pods found": "No pods found", "No polices found": "No polices found", "No policies available for selection.": "No policies available for selection.", @@ -2383,7 +2379,6 @@ "Placement YAML": "Placement YAML", "PlacementDecisions": "PlacementDecisions", "Placements": "Placements", - "Placements define the target clusters that must subscribe to a ClusterSet where subscriptions and application sets are delivered. This is done by cluster name, cluster resource annotation(s), or cluster resource label(s).": "Placements define the target clusters that must subscribe to a ClusterSet where subscriptions and application sets are delivered. This is done by cluster name, cluster resource annotation(s), or cluster resource label(s).", "Platform specific data for the add node pool form will no longer automatically populate after the last node pool is removed.": "Platform specific data for the add node pool form will no longer automatically populate after the last node pool is removed.", "Plugin requeue time in seconds": "Plugin requeue time in seconds", "Plus": "Plus", @@ -3754,7 +3749,6 @@ "You don't have any credentials yet": "You don't have any credentials yet", "You don't have any discovered clusters yet": "You don't have any discovered clusters yet", "You don't have any node pools yet": "You don't have any node pools yet", - "You don't have any placements": "You don't have any placements", "You don't have any placements yet": "You don't have any placements yet", "You don't have any policies yet": "You don't have any policies yet", "You don't have any policies.": "You don't have any policies.", diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index 62c0a12bf40..97002bbcfe9 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -175,7 +175,7 @@ export interface Settings { ansibleIntegration?: 'enabled' | 'disabled' singleNodeOpenshift?: 'enabled' | 'disabled' awsPrivateWizardStep?: 'enabled' | 'disabled' - enhancedPlacement?: 'enabled' | 'disabled' + globalSearchFeatureFlag?: 'enabled' | 'disabled' APP_ARGO_SEARCH_RESULT_LIMIT?: string diff --git a/frontend/src/routes/Applications/AdvancedConfiguration.test.tsx b/frontend/src/routes/Applications/AdvancedConfiguration.test.tsx index 98a3152829c..e99364dc8a4 100644 --- a/frontend/src/routes/Applications/AdvancedConfiguration.test.tsx +++ b/frontend/src/routes/Applications/AdvancedConfiguration.test.tsx @@ -9,7 +9,6 @@ import { namespacesState, placementDecisionsState, placementsState, - settingsState, subscriptionsState, } from '../../atoms' import { nockIgnoreApiPaths, nockIgnoreRBAC, nockSearch } from '../../lib/nock-util' @@ -272,13 +271,7 @@ describe('advanced configuration page', () => { test('should render deprecation Alert', async () => { render( - { - snapshot.set(settingsState, { - enhancedPlacement: 'enabled', - }) - }} - > + @@ -298,11 +291,6 @@ describe('advanced configuration page', () => { render() await clickByTestId('channels') }) - - test('should click placement option', async () => { - render() - await clickByTestId('placements') - }) }) describe('getPlacementDecisionClusterCount', () => { @@ -424,21 +412,4 @@ describe('Export from application tables', () => { /^applicationadvancedconfiguration-channels-[\d]+\.csv$/ ) }) - - test('export button should produce a file for download for placements', async () => { - render() - const { blobConstructorSpy, createElementSpy } = getCSVExportSpies() - - //download for placements - await clickByLabel('export-search-result') - await clickByText('Export all to CSV') - - expect(blobConstructorSpy).toHaveBeenCalledWith( - ['Name,Namespace,Clusters,Created\n"test-placement","default","None","2024-06-28T03:18:48.000Z"'], - { type: 'text/csv' } - ) - expect(getCSVDownloadLink(createElementSpy)?.value.download).toMatch( - /^applicationadvancedconfiguration-placements-[\d]+\.csv$/ - ) - }) }) diff --git a/frontend/src/routes/Applications/AdvancedConfiguration.tsx b/frontend/src/routes/Applications/AdvancedConfiguration.tsx index 224b46e09a6..d6342b2b770 100644 --- a/frontend/src/routes/Applications/AdvancedConfiguration.tsx +++ b/frontend/src/routes/Applications/AdvancedConfiguration.tsx @@ -26,9 +26,7 @@ import { ChannelDefinition, ChannelKind, IResource, - PlacementApiVersionBeta, PlacementDecision, - PlacementDefinition, PlacementKind, Subscription, SubscriptionApiVersion, @@ -56,21 +54,12 @@ export interface AdvancedConfigurationPageProps { export default function AdvancedConfiguration(props: AdvancedConfigurationPageProps) { const { t } = useTranslation() - const { - applicationsState, - channelsState, - placementsState, - placementDecisionsState, - subscriptionsState, - settingsState, - } = useSharedAtoms() + const { applicationsState, channelsState, placementDecisionsState, subscriptionsState } = useSharedAtoms() const applications = useRecoilValue(applicationsState) const channels = useRecoilValue(channelsState) - const placements = useRecoilValue(placementsState) const placementDecisions = useRecoilValue(placementDecisionsState) const subscriptions = useRecoilValue(subscriptionsState) - const settings = useRecoilValue(settingsState) const subscriptionsWithoutLocal = subscriptions.filter((subscription) => { return !_.endsWith(subscription.metadata.name, '-local') @@ -81,10 +70,8 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr const navigate = useNavigate() const [canDeleteSubscription, setCanDeleteSubscription] = useState(false) const [canDeleteChannel, setCanDeleteChannel] = useState(false) - const [canDeletePlacement, setCanDeletePlacement] = useState(false) const ChanneltableItems: IResource[] = [] const SubscriptiontableItems: IResource[] = [] - const PlacementTableItems: IResource[] = [] const localHubName = useLocalHubName() @@ -104,14 +91,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr return () => canDeleteChannelPromise.abort() }, []) - useEffect(() => { - const canDeletePlacementPromise = canUser('delete', PlacementDefinition) - canDeletePlacementPromise.promise - .then((result) => setCanDeletePlacement(result.status?.allowed!)) - .catch((err) => console.error(err)) - return () => canDeletePlacementPromise.abort() - }, []) - const editLink = useCallback( function editLink(params: { resource: any; kind: string; apiversion: string }) { const { resource, kind, apiversion } = params @@ -187,7 +166,7 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr const transformedObject = { transformed: {}, } - let clusterCount = { + const clusterCount = { localPlacement: false, remoteCount: 0, } @@ -249,12 +228,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr } break } - case 'Placement': { - clusterCount = getPlacementDecisionClusterCount(tableItem, clusterCount, placementDecisions, localHubName) - const clusterString = getClusterCountString(t, clusterCount) - _.set(transformedObject.transformed, 'clusterCount', clusterString) - break - } } // Cannot add properties directly to objects in typescript return { ...tableItem, ...transformedObject } @@ -266,7 +239,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr subscriptionsWithoutLocal.forEach((subscription) => { SubscriptiontableItems.push(generateTransformData(subscription)) }) - placements.forEach((placement) => PlacementTableItems.push(generateTransformData(placement))) const getRowActionResolver = (item: IResource) => { const kind = _.get(item, 'kind').toLowerCase() @@ -290,12 +262,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr searchActionLabel = t('Search channel') deleteActionLabel = t('Delete channel') break - case PlacementKind: - canDeleteResource = canDeletePlacement - editActionLabel = t('Edit placement') - searchActionLabel = t('Search placement') - deleteActionLabel = t('Delete placement') - break } // edit @@ -373,55 +339,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr return '' } - const placementColumns = useMemo[]>( - () => [ - { - header: t('Name'), - cell: (resource) => { - return editLink({ - resource, - kind: 'Placement', - apiversion: _.get(resource, 'apiVersion') || PlacementApiVersionBeta, - }) - }, - sort: 'metadata.name', - search: 'metadata.name', - exportContent: (resource) => resource.metadata?.name, - }, - { - header: t('Namespace'), - cell: 'metadata.namespace', - sort: 'metadata.namespace', - exportContent: (resource) => resource.metadata?.namespace, - }, - { - header: t('Clusters'), - cell: 'transformed.clusterCount', - sort: 'transformed.clusterCount', - tooltip: t( - 'Displays the number of remote and local clusters where resources are deployed because of the placement.' - ), - exportContent: (resource) => { - const clusters = _.get(resource, 'transformed.clusterCount') - return clusters - }, - }, - { - header: t('Created'), - cell: (resource) => { - return {getResourceTimestamp(resource, 'metadata.creationTimestamp')} - }, - sort: 'metadata.creationTimestamp', - exportContent: (resource) => { - if (resource.metadata?.creationTimestamp) { - return getISOStringTimestamp(resource.metadata?.creationTimestamp) - } - }, - }, - ], - [t, editLink] - ) - const table = { subscriptions: { columns: useMemo[]>( @@ -693,15 +610,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr items: ChanneltableItems, rowActionResolver: getRowActionResolver, }, - ...(settings.enhancedPlacement !== 'enabled' - ? { - placements: { - columns: placementColumns, - items: PlacementTableItems, - rowActionResolver: getRowActionResolver, - }, - } - : {}), } const keyFn = useCallback( @@ -734,14 +642,6 @@ export default function AdvancedConfiguration(props: AdvancedConfigurationPagePr 'Channels point to repositories where Kubernetes resources are stored, such as Git, Helm chart, or object storage repositories. Channels support multiple subscriptions from multiple targets.' )} /> - {settings.enhancedPlacement !== 'enabled' && ( - - )} - {settings.enhancedPlacement === 'enabled' && ( - - {t('Learn more')} - - } - > - {t( - 'This page will be removed in a future release. Placements will move to a central location under Infrastructure > Clusters > Placements. You can also view placement details directly within individual applications or policies.' - )} - - )} + + {t('Learn more')} + + } + > + {t( + 'This page will be removed in a future release. Placements will move to a central location under Infrastructure > Clusters > Placements. You can also view placement details directly within individual applications or policies.' + )} + diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.test.tsx b/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.test.tsx index 28642b9cf6b..f2f6f2191ad 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.test.tsx +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.test.tsx @@ -2,13 +2,7 @@ import { render } from '@testing-library/react' import { MemoryRouter, Outlet, Route, Routes } from 'react-router-dom-v5-compat' import { RecoilRoot } from 'recoil' -import { - channelsState, - managedClustersState, - namespacesState, - settingsState, - subscriptionsState, -} from '../../../../atoms' +import { channelsState, managedClustersState, namespacesState, subscriptionsState } from '../../../../atoms' import { nockIgnoreApiPaths, nockIgnoreRBAC, nockRBAC } from '../../../../lib/nock-util' import { clickByText, waitForNock, waitForText } from '../../../../lib/test-util' @@ -657,9 +651,6 @@ describe('Overview Tab', () => { snapshot.set(subscriptionsState, mockSubscriptions) snapshot.set(channelsState, mockChannels) snapshot.set(managedClustersState, mockManagedClusters) - snapshot.set(settingsState, { - enhancedPlacement: 'enabled', - }) }} > diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.tsx b/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.tsx index efc599163b2..efbcfd3fca3 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.tsx +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationDetails/ApplicationDetails.tsx @@ -72,11 +72,10 @@ const clusterResourceStatusTooltipOther = (t: TFunction) => t('Status of resourc export function ApplicationDetailsPageContent() { const { applicationData } = useApplicationDetailsContext() const { t } = useTranslation() - const { channelsState, namespacesState, subscriptionsState, settingsState } = useSharedAtoms() + const { channelsState, namespacesState, subscriptionsState } = useSharedAtoms() const channels = useRecoilValue(channelsState) const subscriptions = useRecoilValue(subscriptionsState) const namespaces = useRecoilValue(namespacesState) - const settings = useRecoilValue(settingsState) const localCluster = useLocalHubName() const [modalProps, setModalProps] = useState({ open: false, @@ -282,14 +281,10 @@ export function ApplicationDetailsPageContent() { ), }, - ...(settings.enhancedPlacement === 'enabled' - ? [ - { - key: t('Placement'), - value: , - }, - ] - : []), + { + key: t('Placement'), + value: , + }, ] } else { /////////////////////////// subscription items ////////////////////////////////////////////// diff --git a/frontend/src/routes/Applications/components/ToggleSelector.tsx b/frontend/src/routes/Applications/components/ToggleSelector.tsx index c172eb13717..ce5a60ad77c 100644 --- a/frontend/src/routes/Applications/components/ToggleSelector.tsx +++ b/frontend/src/routes/Applications/components/ToggleSelector.tsx @@ -5,7 +5,7 @@ import _ from 'lodash' import queryString from 'query-string' import { TFunction } from 'react-i18next' import { Link, useNavigate } from 'react-router-dom-v5-compat' -import { useRecoilValue, useSharedAtoms } from '~/shared-recoil' + import { DOC_LINKS, ViewDocumentationLink } from '../../../lib/doc-util' import { rbacCreate, useIsAnyNamespaceAuthorized } from '../../../lib/rbac-util' import { NavigationPath } from '../../../NavigationPath' @@ -20,20 +20,14 @@ export interface IToggleSelectorProps { t: TFunction defaultToggleOption?: ApplicationToggleOptions } -export type ApplicationToggleOptions = 'subscriptions' | 'channels' | 'placements' +export type ApplicationToggleOptions = 'subscriptions' | 'channels' export function ToggleSelector(props: IToggleSelectorProps) { - const { settingsState } = useSharedAtoms() - const settings = useRecoilValue(settingsState) - const t = props.t const defaultOption = props.defaultToggleOption ?? 'subscriptions' const options = [ { id: 'subscriptions', title: t('Subscriptions'), emptyMessage: t("You don't have any subscriptions yet") }, { id: 'channels', title: t('Channels'), emptyMessage: t("You don't have any channels") }, - ...(settings.enhancedPlacement !== 'enabled' - ? [{ id: 'placements' as const, title: t('Placements'), emptyMessage: t("You don't have any placements") }] - : []), ] as const const canCreateApplication = useIsAnyNamespaceAuthorized(rbacCreate(ApplicationDefinition)) const selectedId = getSelectedId({ location, options, defaultOption, queryParam: 'resources' }) diff --git a/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.test.tsx b/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.test.tsx index 8a45c5712f6..1a0bc8939e2 100644 --- a/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.test.tsx +++ b/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.test.tsx @@ -9,7 +9,6 @@ import { placementsState, policiesState, policySetsState, - settingsState, } from '../../../../atoms' import { nockIgnoreApiPaths, nockIgnoreRBAC } from '../../../../lib/nock-util' import { waitForText } from '../../../../lib/test-util' @@ -56,7 +55,6 @@ describe('Policy Details Results', () => { snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) snapshot.set(policiesState, [mockPolicy[1]]) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -100,7 +98,6 @@ describe('Policy Details Results', () => { snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) snapshot.set(policiesState, propagatedPolicies) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -127,7 +124,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -156,7 +152,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -176,43 +171,6 @@ describe('Policy Details Results', () => { await waitForText(/Pending on \d+ cluster/) }) - test('Should render Policy Details Page with placement table when feature flag is disabled', async () => { - const context: PolicyDetailsContext = { policy: mockPolicy[0] } - const { container } = render( - { - snapshot.set(placementsState, mockPlacements) - snapshot.set(policySetsState, [mockPolicySets[0]]) - snapshot.set(placementBindingsState, mockPlacementBindings) - snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(policiesState, [mockPolicy[1]]) - // Feature flag disabled - should show old UI with placement table - snapshot.set(settingsState, { enhancedPlacement: 'disabled' }) - }} - > - - - }> - } /> - - - - - ) - - // wait page load - await waitForText('policy-set-with-1-placement-policy') - - // verify old UI with placement table is rendered - // Check for table column headers which are unique to the table - const tableHeaders = container.querySelectorAll('th') - const headerTexts = Array.from(tableHeaders).map((th) => th.textContent) - expect(headerTexts).toContain('Name') - expect(headerTexts).toContain('Kind') - expect(headerTexts).toContain('Clusters') - expect(headerTexts).toContain('Violations') - }) - test('Should expand cluster violations when clicking "show more" button', async () => { const policyWithManyClusters: Policy = { ...mockPolicy[0], @@ -235,7 +193,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -295,7 +252,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -370,7 +326,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, []) snapshot.set(placementBindingsState, [mockPlacementBindingForRule]) snapshot.set(placementDecisionsState, []) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -399,7 +354,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, []) snapshot.set(placementBindingsState, []) snapshot.set(placementDecisionsState, []) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -439,7 +393,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -468,7 +421,6 @@ describe('Policy Details Results', () => { snapshot.set(policySetsState, [mockPolicySets[0]]) snapshot.set(placementBindingsState, mockPlacementBindings) snapshot.set(placementDecisionsState, mockPlacementDecision) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > diff --git a/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.tsx b/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.tsx index f90a516d9fd..970d9970c02 100644 --- a/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.tsx +++ b/frontend/src/routes/Governance/policies/policy-details/PolicyDetailsOverview.tsx @@ -1,15 +1,5 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { - Alert, - Button, - ButtonVariant, - Content, - ContentVariants, - Icon, - LabelGroup, - PageSection, - Stack, -} from '@patternfly/react-core' +import { Button, ButtonVariant, Icon, PageSection } from '@patternfly/react-core' import { BellIcon, CheckCircleIcon, ExclamationCircleIcon, ExclamationTriangleIcon } from '@patternfly/react-icons' import { ReactNode, useCallback, useContext, useMemo, useState } from 'react' import { generatePath, Link } from 'react-router-dom-v5-compat' @@ -28,7 +18,7 @@ import { } from '../../../../resources' import { Metadata } from '../../../../resources/metadata' import { useRecoilValue, useSharedAtoms } from '../../../../shared-recoil' -import { AcmButton, AcmDescriptionList, AcmDrawerContext, AcmTable } from '../../../../ui-components' +import { AcmButton, AcmDescriptionList, AcmDrawerContext } from '../../../../ui-components' import { usePropagatedPolicies } from '../../common/useCustom' import { getPlacementDecisionsForPlacements, @@ -37,8 +27,7 @@ import { getPolicyRemediation, } from '../../common/util' import { AutomationDetailsSidebar } from '../../components/AutomationDetailsSidebar' -import { ClusterPolicyViolationIcons } from '../../components/ClusterPolicyViolations' -import { useGovernanceData } from '../../useGovernanceData' + import { usePolicyDetailsContext } from './PolicyDetailsPage' import { PlacementLinkList } from '../../../Infrastructure/Clusters/Placements/utils' @@ -54,29 +43,14 @@ export default function PolicyDetailsOverview() { const { policy } = usePolicyDetailsContext() const { t } = useTranslation() const { setDrawerContext } = useContext(AcmDrawerContext) - const { - placementBindingsState, - placementDecisionsState, - placementsState, - policyAutomationState, - policySetsState, - settingsState, - } = useSharedAtoms() + const { placementBindingsState, placementDecisionsState, placementsState, policyAutomationState, policySetsState } = + useSharedAtoms() const placements = useRecoilValue(placementsState) const policySets = useRecoilValue(policySetsState) const placementBindings = useRecoilValue(placementBindingsState) const placementDecisions = useRecoilValue(placementDecisionsState) const policyAutomations = useRecoilValue(policyAutomationState) - const settings = useRecoilValue(settingsState) const policies = usePropagatedPolicies(policy) - const govData = useGovernanceData([policy]) - const clusterRiskScore = - govData.clusterRisks.critical + - govData.clusterRisks.high + - govData.clusterRisks.medium + - govData.clusterRisks.low + - govData.clusterRisks.unknown + - govData.clusterRisks.synced const policyAutomationMatch = policyAutomations.find( (pa: PolicyAutomation) => pa.spec.policyRef === policy.metadata.name ) @@ -283,29 +257,10 @@ export default function PolicyDetailsOverview() { key: t('Remediation'), value: getPolicyRemediation(policy, policies), }, - ...(settings.enhancedPlacement !== 'enabled' - ? [ - { - key: t('Cluster violations'), - value: - clusterRiskScore > 0 ? ( - - ) : ( -
- {'No status'} -
- ), - }, - ] - : []), - ...(settings.enhancedPlacement === 'enabled' - ? [ - { - key: t('Cluster violations'), - value: renderPolicyViolations(expandedViolationStatuses, toggleViolationExpanded), - }, - ] - : []), + { + key: t('Cluster violations'), + value: renderPolicyViolations(expandedViolationStatuses, toggleViolationExpanded), + }, ] const rightItems = [ @@ -378,34 +333,28 @@ export default function PolicyDetailsOverview() { ), }, - ...(settings.enhancedPlacement === 'enabled' - ? (() => { - const placementResources = placementMatches.map( - (p) => ({ kind: p.kind, apiVersion: p.apiVersion, metadata: p.metadata }) as Placement - ) - return [ - { - key: t('Placement'), - value: - placementResources.length > 0 ? ( - - ) : ( -
- - - {' '} - {t('No placement selectors found')} -
- ), - }, - ] - })() - : []), + (() => { + const placementResources = placementMatches.map( + (p) => ({ kind: p.kind, apiVersion: p.apiVersion, metadata: p.metadata }) as Placement + ) + return { + key: t('Placement'), + value: + placementResources.length > 0 ? ( + + ) : ( +
+ + + {' '} + {t('No placements found')} +
+ ), + } + })(), ] return { leftItems, rightItems } }, [ - clusterRiskScore, - govData.clusterRisks, policy, policyAutomationMatch, setDrawerContext, @@ -416,198 +365,15 @@ export default function PolicyDetailsOverview() { renderPolicyViolations, expandedViolationStatuses, toggleViolationExpanded, - settings.enhancedPlacement, t, ]) - const placementCols = useMemo( - () => [ - { - header: t('Name'), - cell: 'metadata.name', - sort: 'metadata.name', - }, - { - header: t('Kind'), - cell: 'kind', - sort: 'kind', - }, - { - header: t('Clusters'), - cell: (item: TableData) => { - const decisions = item.status.decisions ?? undefined - if (decisions) { - return decisions.map((decision: { clusterName: string }) => decision.clusterName).length - } - return 0 - }, - }, - { - header: t('Violations'), - cell: (item: TableData) => { - // Gather full cluster list from placementPolicy status - const fullClusterList = item.status.decisions ?? [] - // Gather status list from policy status - const rawStatusList: { - clustername: string - compliant?: string - }[] = item.policy.status?.status ?? [] - // Build lists of clusters, organized by status keys - const clusterList: Record> = {} - fullClusterList.forEach((clusterObj) => { - const statusObject = rawStatusList.filter((status) => status.clustername === clusterObj.clusterName) - // Log error if more than one status is returned since each cluster name should be unique - if (statusObject.length > 1) { - console.error(`Expected one cluster but got ${statusObject.length}:`, statusObject) - } else if (statusObject.length === 0) { - // Push a new cluster object if there is no status found - statusObject.push({ - clustername: clusterObj.clusterName, - compliant: 'nostatus', - }) - } - let compliant = statusObject[0]?.compliant ?? 'nostatus' - compliant = compliant.toLowerCase() - const clusterName = statusObject[0].clustername - // Add cluster to its associated status list in the clusterList object - if (Object.prototype.hasOwnProperty.call(clusterList, compliant)) { - // Each cluster name should be unique, so if one is already present, log an error - if (clusterList[compliant].has(clusterName)) { - console.error(`Unexpected duplicate cluster in '${compliant}' cluster list: ${clusterName}`) - } else { - clusterList[compliant].add(clusterName) - } - } else { - clusterList[compliant] = new Set([clusterName]) - } - }) - // Push lists of clusters along with status icon, heading, and overflow badge - const statusList = [] - for (const status of Object.keys(clusterList)) { - let statusMsg = t(' No status: ') - let icon = - switch (status) { - case 'noncompliant': - statusMsg = t(' Violations: ') - icon = ( - - - - ) - break - case 'compliant': - statusMsg = t(' No violations: ') - icon = ( - - - - ) - break - case 'pending': - statusMsg = ' Pending: ' - icon = ( - - - - ) - break - } - statusList.push( -
- - - {icon} - {statusMsg} - - - - - {Array.from(clusterList[status]).map((cluster: string, index) => { - if (status !== 'nostatus') { - return ( - - - {cluster} - {index < clusterList[status].size - 1 && ', '} - - - ) - } - return ( - - {cluster} - {index < clusterList[status].size - 1 && ', '} - - ) - })} - - -
- ) - } - // If there are no clusters, return a hyphen - if (statusList.length === 0) { - return ( -
- {t('No status')} -
- ) - } - return statusList - }, - }, - ], - [policy.metadata.name, policy.metadata.namespace, t] - ) - return ( {modal !== undefined && modal} - {settings.enhancedPlacement === 'enabled' ? ( -
- -
- ) : ( - -
- -
-
- - {t('Placement')} - - {placementMatches.length > 0 ? ( - - key="cluster-placement-list" - items={placementMatches} - emptyState={undefined} // only shown when there are placement matches - columns={placementCols} - keyFn={(item) => item.metadata.uid!.toString()} - autoHidePagination={true} - /> - ) : ( - - )} -
- - )} +
+ +
) } diff --git a/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.test.tsx b/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.test.tsx index 92106a0c0cf..378d614ab5c 100644 --- a/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.test.tsx +++ b/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.test.tsx @@ -6,7 +6,7 @@ import { AcmDrawerContext } from '../../../../ui-components' import { waitForText } from '../../../../lib/test-util' import { PolicySet, Placement, PlacementBinding } from '../../../../resources' import PolicySetCard from './PolicySetCard' -import { placementBindingsState, placementsState, settingsState } from '../../../../atoms' +import { placementBindingsState, placementsState } from '../../../../atoms' const cardID = 'policyset-test-policy-set-with-1-placement' @@ -361,7 +361,6 @@ describe('Policy Set Card drawer behavior (onSelect vs onViewDetails)', () => { initializeState={(snapshot) => { snapshot.set(placementsState, mockPlacements) snapshot.set(placementBindingsState, mockPlacementBindings) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > @@ -391,7 +390,6 @@ describe('Policy Set Card drawer behavior (onSelect vs onViewDetails)', () => { initializeState={(snapshot) => { snapshot.set(placementsState, mockPlacements) snapshot.set(placementBindingsState, mockPlacementBindings) - snapshot.set(settingsState, { enhancedPlacement: 'enabled' }) }} > {drawerProps.title} diff --git a/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.tsx b/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.tsx index 8eaa599bdaf..2cf4864e999 100644 --- a/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.tsx +++ b/frontend/src/routes/Governance/policy-sets/components/PolicySetCard.tsx @@ -33,10 +33,9 @@ import { getPlacementsForResource } from '../../common/util' function PolicySetDrawerTitle(props: { policySet: PolicySet }) { const { policySet } = props const { t } = useTranslation() - const { placementBindingsState, placementsState, settingsState } = useSharedAtoms() + const { placementBindingsState, placementsState } = useSharedAtoms() const placements = useRecoilValue(placementsState) const placementBindings = useRecoilValue(placementBindingsState) - const settings = useRecoilValue(settingsState) const policySetPlacements = useMemo( () => getPlacementsForResource(policySet, placementBindings, placements), @@ -49,7 +48,7 @@ function PolicySetDrawerTitle(props: { policySet: PolicySet }) {
{`${t('Namespace')}: ${policySet.metadata.namespace}`}
- {settings.enhancedPlacement === 'enabled' && policySetPlacements.length > 0 && ( + {policySetPlacements.length > 0 && (
{`${t('Placement')}: `} {policySetPlacements diff --git a/frontend/src/routes/Infrastructure/Clusters/ClustersPage.tsx b/frontend/src/routes/Infrastructure/Clusters/ClustersPage.tsx index 2d0493a6725..5ba245e2613 100644 --- a/frontend/src/routes/Infrastructure/Clusters/ClustersPage.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/ClustersPage.tsx @@ -6,7 +6,7 @@ import { useTranslation } from '../../../lib/acm-i18next' import { DOC_LINKS } from '../../../lib/doc-util' import { NavigationPath } from '../../../NavigationPath' import { AcmPage, AcmPageHeader, AcmSecondaryNav } from '../../../ui-components' -import { useRecoilValue, useSharedAtoms } from '../../../shared-recoil' + export const PageContext = createContext<{ readonly actions: null | ReactNode setActions: (actions: null | ReactNode) => void @@ -41,9 +41,6 @@ export function ClustersPage() { const [actions, setActions] = useState(undefined) const location = useLocation() const { t } = useTranslation() - const { settingsState } = useSharedAtoms() - const settings = useRecoilValue(settingsState) // featureflag to be removed - const tabs: any[] = [ { key: 'infra-cluster-list', @@ -69,17 +66,13 @@ export function ClustersPage() { isActive: location.pathname.startsWith(NavigationPath.discoveredClusters), to: NavigationPath.discoveredClusters, }, - ] - - if (settings.enhancedPlacement === 'enabled') { - // TODO: remove feature flag - tabs.push({ + { key: 'infra-placements', title: t('Placements'), isActive: location.pathname.startsWith(NavigationPath.placements), to: NavigationPath.placements, - }) - } + }, + ] return ( (false) const [curatorSummaryModalIsOpen, setCuratorSummaryModalIsOpen] = useState(false) const { projects } = useProjects() - const { settingsState, placementsState, placementDecisionsState } = useSharedAtoms() - const settings = useRecoilValue(settingsState) + const { placementsState, placementDecisionsState } = useSharedAtoms() const placements = useRecoilValue(placementsState) const placementDecisions = useRecoilValue(placementDecisionsState) const placementsForCluster = useMemo(() => { @@ -387,7 +386,7 @@ export function ClusterOverviewPageContent() { ? [clusterProperties.automationTemplate] : []), ]), - ...(settings.enhancedPlacement === 'enabled' ? [clusterProperties.placements] : []), + clusterProperties.placements, ] const [isModalOpen, setIsModalOpen] = useState(false) diff --git a/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/EditPlacement.tsx b/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/EditPlacement.tsx index b43a0ee3471..863a853fd76 100644 --- a/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/EditPlacement.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/EditPlacement.tsx @@ -1,6 +1,12 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { EditorValidationStatus, useData, useEditorValidationStatus, useItem } from '@patternfly-labs/react-form-wizard' +import { + EditorValidationStatus, + useData, + useEditorValidationStatus, + useHighlightEditorPath, + useItem, +} from '@patternfly-labs/react-form-wizard' import { SyncEditor, ValidationStatus } from '~/components/SyncEditor/SyncEditor' import { useTranslation } from '~/lib/acm-i18next' import schema from './schema.json' @@ -22,6 +28,7 @@ export function WizardSyncEditor() { const resources = useItem() const { update } = useData() const { setEditorValidationStatus } = useEditorValidationStatus() + const { highlightEditorPath } = useHighlightEditorPath() const { t } = useTranslation() return ( @@ -39,6 +46,7 @@ export function WizardSyncEditor() { editableUidSiblings={true} filters={['*.metadata.managedFields']} immutables={['*.metadata']} + highlightEditorPath={highlightEditorPath} /> ) } diff --git a/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/PlacementWizard.tsx b/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/PlacementWizard.tsx index 0b2e9f54378..0f2e428ef91 100644 --- a/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/PlacementWizard.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/Placements/CreatePlacement/PlacementWizard.tsx @@ -16,7 +16,7 @@ import { useItem, } from '@patternfly-labs/react-form-wizard' import { useValidation } from '~/hooks/useValidation' -import { useRecoilValue, useSharedAtoms } from '~/shared-recoil' + import { IClusterSetBinding } from '~/wizards/common/resources/IClusterSetBinding' import { IPlacement, PlacementKind, PlacementType } from '~/wizards/common/resources/IPlacement' import { NavigationPath } from '~/NavigationPath' @@ -124,8 +124,6 @@ function PlacementStepContent(props: { clusters: IResource[] }) { const { t } = useTranslation() - const { settingsState } = useSharedAtoms() - const settings = useRecoilValue(settingsState) const placement = useItem() as IPlacement const namespace = placement?.metadata?.namespace @@ -146,7 +144,7 @@ function PlacementStepContent(props: { 'ClusterSets failed to load. Verify that there is at least one ClusterSet bound to your selected namespace.' )} hideName - showPlacementPreview={settings.enhancedPlacement === 'enabled'} + showPlacementPreview alertContent={ } - showPlacementPreview={settings.enhancedPlacement === 'enabled'} + showPlacementPreview /> ) : ( diff --git a/frontend/src/wizards/Placement/Placement.tsx b/frontend/src/wizards/Placement/Placement.tsx index 46789a0e7bc..0c50a819460 100644 --- a/frontend/src/wizards/Placement/Placement.tsx +++ b/frontend/src/wizards/Placement/Placement.tsx @@ -115,7 +115,7 @@ export function Placement(props: { const [isTolerationsExpanded, setIsTolerationsExpanded] = useState(true) const featureEnabled = props.showPlacementPreview === true const ownsDebugUI = featureEnabled && !props.placementDebugState - const ownDebugState = usePlacementDebug(placement, ownsDebugUI) + const ownDebugState = usePlacementDebug(placement) const { matched, notMatched, totalClusters, matchedCount, error } = props.placementDebugState ?? ownDebugState const { t } = useTranslation() diff --git a/frontend/src/wizards/Placement/PlacementSection.test.tsx b/frontend/src/wizards/Placement/PlacementSection.test.tsx index 8ff69554539..bdf32d22d95 100644 --- a/frontend/src/wizards/Placement/PlacementSection.test.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.test.tsx @@ -141,8 +141,8 @@ describe('PlacementSection', () => { expect(screen.getByTestId('placement-bindings')).toBeInTheDocument() }) - it('passes showPlacementPreview when enhancedPlacement is enabled', () => { - mockSettings = { enhancedPlacement: 'enabled' } + it('passes showPlacementPreview', () => { + mockSettings = {} mockResources = [ { kind: PlacementKind, @@ -345,8 +345,8 @@ describe('PlacementSection', () => { expect(screen.getByTestId('placement-component')).toBeInTheDocument() }) - it('sets footer content when enhancedPlacement is enabled with single placement', () => { - mockSettings = { enhancedPlacement: 'enabled' } + it('sets footer content with single placement', () => { + mockSettings = {} mockResources = [ { kind: PlacementKind, @@ -366,8 +366,8 @@ describe('PlacementSection', () => { expect(mockSetFooterContent).toHaveBeenCalled() }) - it('renders review step content when displayMode is not Step and enhancedPlacement enabled', () => { - mockSettings = { enhancedPlacement: 'enabled' } + it('renders review step content when displayMode is not Step', () => { + mockSettings = {} mockDisplayMode = 1 mockResources = [ { @@ -389,7 +389,7 @@ describe('PlacementSection', () => { }) it('renders review step with label expressions and tolerations', () => { - mockSettings = { enhancedPlacement: 'enabled' } + mockSettings = {} mockDisplayMode = 1 mockResources = [ { @@ -445,7 +445,7 @@ describe('PlacementSection', () => { }) it('renders review step error alert when debug has error', () => { - mockSettings = { enhancedPlacement: 'enabled' } + mockSettings = {} mockDisplayMode = 1 mockResources = [ { diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index 992517277af..b9997a79043 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -44,7 +44,6 @@ import { useTranslation } from '../../lib/acm-i18next' import { NavigationPath } from '../../NavigationPath' import { usePlacementDebug } from './usePlacementDebug' import { MatchedClustersModal } from './MatchedClustersModal' -import { useRecoilValue, useSharedAtoms } from '../../shared-recoil' export function PlacementSection(props: { bindingSubjectKind: string @@ -63,8 +62,6 @@ export function PlacementSection(props: { const resources = useItem() as IResource[] const editMode = useEditMode() const displayMode = useDisplayMode() - const { settingsState } = useSharedAtoms() - const settings = useRecoilValue(settingsState) const [isMatchedClustersModalOpen, setIsMatchedClustersModalOpen] = useState(false) const [placementCount, setPlacementCount] = useState(0) @@ -165,19 +162,14 @@ export function PlacementSection(props: { return resources?.find((resource) => resource.kind === PlacementKind) as IPlacement | undefined }, [resources]) - const debugState = usePlacementDebug(currentPlacement, settings.enhancedPlacement === 'enabled') + const debugState = usePlacementDebug(currentPlacement) const { matched, notMatched, matchedCount, totalClusters, error } = debugState const setFooterContent = useSetFooterContent() const openMatchedModal = useCallback(() => setIsMatchedClustersModalOpen(true), []) useEffect(() => { - if ( - settings.enhancedPlacement === 'enabled' && - placementCount === 1 && - currentPlacement && - displayMode === DisplayMode.Step - ) { + if (placementCount === 1 && currentPlacement && displayMode === DisplayMode.Step) { const matchedLabel = matchedCount === undefined ? '-' @@ -202,7 +194,6 @@ export function PlacementSection(props: { } return () => setFooterContent(undefined) }, [ - settings.enhancedPlacement, placementCount, currentPlacement, displayMode, @@ -223,7 +214,7 @@ export function PlacementSection(props: { clusterSetBindings={props.existingClusterSetBindings} bindingKind={props.bindingSubjectKind} clusters={props.clusters} - showPlacementPreview={settings.enhancedPlacement === 'enabled'} + showPlacementPreview /> ) : null} } - showPlacementPreview={settings.enhancedPlacement === 'enabled'} + showPlacementPreview placementDebugState={debugState} /> @@ -304,84 +295,79 @@ export function PlacementSection(props: { )} {/* Review step content */} - {settings.enhancedPlacement === 'enabled' && - displayMode !== DisplayMode.Step && - placementCount === 1 && - currentPlacement && ( -
- {/* Placement info alert */} - {error ? ( - - - - ) : ( - matchedCount !== undefined && ( - 0 ? 'info' : 'warning'} - isInline - title={ - matchedCount > 0 - ? t('{{matched}} of {{total}} clusters matched by placement', { - matched: matchedCount, - total: totalClusters, - }) - : t( - 'No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.' - ) - } - /> - ) - )} - - {/* Label expressions and tolerations */} - {(currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions - ?.length || - currentPlacement.spec?.tolerations?.length) && ( -
-

{t('Label expressions and tolerations')}

-
- {/* Label expressions */} - {currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions - ?.length && ( -
- {t('Label expressions')}: - - {currentPlacement.spec.predicates[0].requiredClusterSelector.labelSelector.matchExpressions.map( - (expr, idx) => ( - - - {expr.values && expr.values.length > 0 && } - - ) - )} - -
- )} + {displayMode !== DisplayMode.Step && placementCount === 1 && currentPlacement && ( +
+ {/* Placement info alert */} + {error ? ( + + + + ) : ( + matchedCount !== undefined && ( + 0 ? 'info' : 'warning'} + isInline + title={ + matchedCount > 0 + ? t('{{matched}} of {{total}} clusters matched by placement', { + matched: matchedCount, + total: totalClusters, + }) + : t( + 'No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.' + ) + } + /> + ) + )} - {/* Tolerations */} - {currentPlacement.spec?.tolerations?.length && ( -
- {t('Tolerations')}: - - {currentPlacement.spec.tolerations.map((toleration, idx) => ( + {/* Label expressions and tolerations */} + {(!!currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions + ?.length || + (currentPlacement.spec?.tolerations && currentPlacement.spec.tolerations.length > 0)) && ( +
+

{t('Label expressions and tolerations')}

+
+ {/* Label expressions */} + {!!currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions + ?.length && ( +
+ {t('Label expressions')}: + + {currentPlacement.spec.predicates[0].requiredClusterSelector.labelSelector.matchExpressions.map( + (expr, idx) => ( - - - {toleration.value && } - {toleration.effect && } - {toleration.tolerationSeconds != null && ( - - )} + + {expr.values && expr.values.length > 0 && } - ))} - -
- )} -
+ ) + )} + +
+ )} + + {/* Tolerations */} + {currentPlacement.spec?.tolerations && currentPlacement.spec.tolerations.length > 0 && ( +
+ {t('Tolerations')}: + + {currentPlacement.spec.tolerations.map((toleration, idx) => ( + + + + {toleration.value && } + {toleration.effect && } + {toleration.tolerationSeconds != null && } + + ))} + +
+ )}
- )} -
- )} +
+ )} +
+ )} { jest.useRealTimers() }) - it('returns empty state when disabled', () => { - const { result } = renderHook(() => usePlacementDebug(mockPlacement, false)) - - expect(result.current.matchedCount).toBeUndefined() - expect(result.current.loading).toBe(false) - expect(result.current.matched).toEqual([]) - expect(result.current.notMatched).toEqual([]) - expect(result.current.error).toBeUndefined() - }) - it('returns empty state when placement is undefined', () => { const { result } = renderHook(() => usePlacementDebug(undefined)) @@ -72,7 +62,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result } = renderHook(() => usePlacementDebug(mockPlacement)) // After render but before debounce fires, should be loading expect(result.current.loading).toBe(true) @@ -101,7 +91,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result } = renderHook(() => usePlacementDebug(mockPlacement)) act(() => { jest.advanceTimersByTime(500) @@ -130,7 +120,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result } = renderHook(() => usePlacementDebug(mockPlacement)) act(() => { jest.advanceTimersByTime(500) @@ -153,7 +143,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result } = renderHook(() => usePlacementDebug(mockPlacement)) act(() => { jest.advanceTimersByTime(500) @@ -181,7 +171,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result } = renderHook(() => usePlacementDebug(mockPlacement)) act(() => { jest.advanceTimersByTime(500) @@ -202,7 +192,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result: first, unmount } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result: first, unmount } = renderHook(() => usePlacementDebug(mockPlacement)) act(() => { jest.advanceTimersByTime(500) @@ -215,7 +205,7 @@ describe('usePlacementDebug', () => { expect(first.current.matched).toEqual(['cluster1', 'cluster2']) unmount() - const { result: second } = renderHook(() => usePlacementDebug(mockPlacement, true)) + const { result: second } = renderHook(() => usePlacementDebug(mockPlacement)) expect(second.current.matched).toEqual(['cluster1', 'cluster2']) expect(second.current.matchedCount).toBe(2) @@ -228,7 +218,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result, rerender } = renderHook(({ placement }) => usePlacementDebug(placement, true), { + const { result, rerender } = renderHook(({ placement }) => usePlacementDebug(placement), { initialProps: { placement: mockPlacement }, }) @@ -258,7 +248,7 @@ describe('usePlacementDebug', () => { abort: jest.fn(), }) - const { result, rerender } = renderHook(({ placement }) => usePlacementDebug(placement, true), { + const { result, rerender } = renderHook(({ placement }) => usePlacementDebug(placement), { initialProps: { placement: mockPlacement }, }) diff --git a/frontend/src/wizards/Placement/usePlacementDebug.ts b/frontend/src/wizards/Placement/usePlacementDebug.ts index ffa563d14a9..673cc3852bc 100644 --- a/frontend/src/wizards/Placement/usePlacementDebug.ts +++ b/frontend/src/wizards/Placement/usePlacementDebug.ts @@ -65,11 +65,11 @@ function mapDebugResult(result: PlacementDebugResult): PlacementDebugState { } } -export function usePlacementDebug(placement: IPlacement | undefined, enabled = true): PlacementDebugState { +export function usePlacementDebug(placement: IPlacement | undefined): PlacementDebugState { const specKey = placement ? JSON.stringify({ metadata: placement.metadata, spec: placement.spec }) : undefined const [state, setState] = useState(() => { - if (enabled && specKey && specKey === cachedSpecKey && cachedState) { + if (specKey && specKey === cachedSpecKey && cachedState) { return cachedState } return EMPTY_STATE @@ -111,7 +111,7 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t useEffect(() => { const debouncedFetch = debouncedFetchRef.current - if (!enabled || !specKey || !placement) { + if (!specKey || !placement) { setState(EMPTY_STATE) return } @@ -131,7 +131,7 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t // placement is intentionally omitted — specKey is derived from it and // serves as the sole cache/identity key. Including placement would cause // spurious re-fetches on every render due to referential inequality. - }, [specKey, enabled]) // eslint-disable-line react-hooks/exhaustive-deps + }, [specKey]) // eslint-disable-line react-hooks/exhaustive-deps return state } From eebc1a052ff0259dc4bf992e8862172093dd053b Mon Sep 17 00:00:00 2001 From: fxiang1 Date: Tue, 28 Apr 2026 17:01:42 -0400 Subject: [PATCH 2/3] CodeRabbit comments and fix unit tests Signed-off-by: fxiang1 --- frontend/src/lib/nock-util.ts | 8 ++++++++ .../CreatePullApplicationSet.test.tsx | 2 ++ .../CreatePushApplicationSet.test.tsx | 2 ++ .../src/routes/Governance/policies/CreatePolicy.test.tsx | 3 ++- .../Governance/policy-sets/CreatePolicySet.test.tsx | 3 ++- .../routes/Governance/policy-sets/EditPolicySet.test.tsx | 3 ++- .../ClusterOverview/ClusterOverview.test.tsx | 2 +- frontend/src/wizards/Argo/ArgoWizard.test.tsx | 9 +++++++++ .../Governance/PolicySet/PolicySetWizard.test.tsx | 5 +++++ frontend/src/wizards/Placement/Placement.tsx | 2 +- 10 files changed, 34 insertions(+), 5 deletions(-) diff --git a/frontend/src/lib/nock-util.ts b/frontend/src/lib/nock-util.ts index b3630726fa6..f18770bcc73 100644 --- a/frontend/src/lib/nock-util.ts +++ b/frontend/src/lib/nock-util.ts @@ -398,6 +398,14 @@ export function nockIgnoreOperatorCheck(noAnsible?: boolean) { .reply(200, noAnsible ? mockOperatorCheckResponseNoAnsible : mockOperatorCheckResponse) } +export function nockIgnorePlacementDebug() { + return nocked(process.env.JEST_DEFAULT_HOST as string) + .persist() + .post('/placement-debug') + .optionally() + .reply(200, { aggregatedScores: [], filteredPipelineResults: [] }) +} + export function nockIgnoreClusterVersion(version = '4.20.0') { return nocked(process.env.JEST_DEFAULT_HOST as string) .persist() diff --git a/frontend/src/routes/Applications/CreateArgoApplication/CreatePullApplicationSet.test.tsx b/frontend/src/routes/Applications/CreateArgoApplication/CreatePullApplicationSet.test.tsx index 80eb52d2121..e1319a34fb0 100644 --- a/frontend/src/routes/Applications/CreateArgoApplication/CreatePullApplicationSet.test.tsx +++ b/frontend/src/routes/Applications/CreateArgoApplication/CreatePullApplicationSet.test.tsx @@ -21,6 +21,7 @@ import { nockGet, nockIgnoreApiPaths, nockIgnoreOperatorCheck, + nockIgnorePlacementDebug, nockList, } from '~/lib/nock-util' import { @@ -396,6 +397,7 @@ describe('Create Argo Application Set', () => { beforeEach(() => { nockIgnoreApiPaths() nockIgnoreOperatorCheck() + nockIgnorePlacementDebug() }) const AddApplicationSet = () => { return ( diff --git a/frontend/src/routes/Applications/CreateArgoApplication/CreatePushApplicationSet.test.tsx b/frontend/src/routes/Applications/CreateArgoApplication/CreatePushApplicationSet.test.tsx index 9cbf9daa0a3..ec3c3ebcbc1 100644 --- a/frontend/src/routes/Applications/CreateArgoApplication/CreatePushApplicationSet.test.tsx +++ b/frontend/src/routes/Applications/CreateArgoApplication/CreatePushApplicationSet.test.tsx @@ -20,6 +20,7 @@ import { nockGet, nockIgnoreApiPaths, nockIgnoreOperatorCheck, + nockIgnorePlacementDebug, nockList, } from '~/lib/nock-util' import { @@ -328,6 +329,7 @@ describe('Create Argo Application Set', () => { beforeEach(() => { nockIgnoreApiPaths() nockIgnoreOperatorCheck() + nockIgnorePlacementDebug() }) const AddApplicationSet = () => { return ( diff --git a/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx b/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx index 23050d0c49b..881065b5677 100755 --- a/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx +++ b/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx @@ -10,7 +10,7 @@ import { managedClusterSetBindingsState, managedClusterSetsState, } from '../../../atoms' -import { nockCreate, nockIgnoreApiPaths, nockIgnoreRBAC } from '../../../lib/nock-util' +import { nockCreate, nockIgnoreApiPaths, nockIgnorePlacementDebug, nockIgnoreRBAC } from '../../../lib/nock-util' import { clickByText, waitForNocks, waitForNotText, waitForText } from '../../../lib/test-util' import { NavigationPath } from '../../../NavigationPath' import { CreatePolicy } from './CreatePolicy' @@ -54,6 +54,7 @@ describe('Create Policy Page', () => { beforeEach(async () => { nockIgnoreRBAC() nockIgnoreApiPaths() + nockIgnorePlacementDebug() }) test('can create policy', async () => { diff --git a/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx b/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx index 55f367ee8b4..ec590bc65d8 100644 --- a/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx +++ b/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx @@ -3,7 +3,7 @@ import { render, screen } from '@testing-library/react' import { MemoryRouter, Route, Routes } from 'react-router-dom-v5-compat' import { RecoilRoot } from 'recoil' import { managedClustersState, namespacesState, policiesState, policySetsState } from '../../../atoms' -import { nockIgnoreRBAC, nockCreate, nockIgnoreApiPaths } from '../../../lib/nock-util' +import { nockIgnoreRBAC, nockCreate, nockIgnoreApiPaths, nockIgnorePlacementDebug } from '../../../lib/nock-util' import { waitForNocks, waitForText } from '../../../lib/test-util' import { NavigationPath } from '../../../NavigationPath' import { CreatePolicySet } from './CreatePolicySet' @@ -34,6 +34,7 @@ describe('Create Policy Page', () => { beforeEach(async () => { nockIgnoreRBAC() nockIgnoreApiPaths() + nockIgnorePlacementDebug() }) test('can create policy set', async () => { diff --git a/frontend/src/routes/Governance/policy-sets/EditPolicySet.test.tsx b/frontend/src/routes/Governance/policy-sets/EditPolicySet.test.tsx index 6fde5a21467..fdaeb1a1dad 100644 --- a/frontend/src/routes/Governance/policy-sets/EditPolicySet.test.tsx +++ b/frontend/src/routes/Governance/policy-sets/EditPolicySet.test.tsx @@ -12,7 +12,7 @@ import { policiesState, policySetsState, } from '../../../atoms' -import { nockIgnoreApiPaths, nockIgnoreRBAC, nockPatch } from '../../../lib/nock-util' +import { nockIgnoreApiPaths, nockIgnorePlacementDebug, nockIgnoreRBAC, nockPatch } from '../../../lib/nock-util' import { NavigationPath } from '../../../NavigationPath' import { EditPolicySet } from './EditPolicySet' import { @@ -59,6 +59,7 @@ describe('Edit Policy Set Page', () => { beforeEach(async () => { nockIgnoreRBAC() nockIgnoreApiPaths() + nockIgnorePlacementDebug() }) test('should render edit policy page', async () => { diff --git a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterOverview/ClusterOverview.test.tsx b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterOverview/ClusterOverview.test.tsx index 205aefff7b8..c8c18b8425a 100644 --- a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterOverview/ClusterOverview.test.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterOverview/ClusterOverview.test.tsx @@ -494,7 +494,7 @@ describe('ClusterOverview channel display for hypershift clusters', () => { // Verify cluster name is shown await waitForText(mockHypershiftClusterWithOCPVersion.name) // Verify channel displays "-" when no channel is defined - await waitForText('-') + await waitForText('-', true) }) it('should display channel when it is defined for hypershift cluster', async () => { diff --git a/frontend/src/wizards/Argo/ArgoWizard.test.tsx b/frontend/src/wizards/Argo/ArgoWizard.test.tsx index e175d4db818..d01ab2ee5b5 100644 --- a/frontend/src/wizards/Argo/ArgoWizard.test.tsx +++ b/frontend/src/wizards/Argo/ArgoWizard.test.tsx @@ -11,6 +11,7 @@ import { nockArgoGitPathTree, nockIgnoreApiPaths, nockIgnoreOperatorCheck, + nockIgnorePlacementDebug, } from '../../lib/nock-util' import { createClusterVersionMock } from '../../lib/test-util' @@ -130,6 +131,7 @@ describe('ArgoWizard tests', () => { //===================================================================== test('create git', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() const url = 'https://github.com/fxiang1/app-samples' render() @@ -240,6 +242,7 @@ describe('ArgoWizard tests', () => { test('various auto sync options', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() const url = 'https://github.com/fxiang1/app-samples' render() @@ -355,6 +358,7 @@ describe('ArgoWizard tests', () => { // HELM //===================================================================== test('create helm', async () => { + nockIgnorePlacementDebug() render() //===================================================================== @@ -454,6 +458,7 @@ describe('ArgoWizard tests', () => { //===================================================================== test('create git pull model', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() const url = 'https://github.com/fxiang1/app-samples' render() @@ -536,6 +541,7 @@ describe('ArgoWizard tests', () => { describe('gitGeneratorRepos extraction from applicationSets', () => { test('extracts git generator URLs, versions, and paths from regular git generators', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() render() // Navigate to general page @@ -558,6 +564,7 @@ describe('ArgoWizard tests', () => { test('extracts git generator info from matrix generators', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() render() // Navigate to general page @@ -580,6 +587,7 @@ describe('ArgoWizard tests', () => { test('deduplicates URLs, versions, and paths from multiple applicationSets', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() // Include both an appset and one with duplicate values render( { test('combines git info from multiple applicationSets', async () => { nockIgnoreApiPaths() + nockIgnorePlacementDebug() render( { + beforeEach(() => { + nockIgnorePlacementDebug() + }) + test('can show correct cluster sets dropdown', async () => { const { container } = render() diff --git a/frontend/src/wizards/Placement/Placement.tsx b/frontend/src/wizards/Placement/Placement.tsx index 0c50a819460..f68dd1ac344 100644 --- a/frontend/src/wizards/Placement/Placement.tsx +++ b/frontend/src/wizards/Placement/Placement.tsx @@ -115,7 +115,7 @@ export function Placement(props: { const [isTolerationsExpanded, setIsTolerationsExpanded] = useState(true) const featureEnabled = props.showPlacementPreview === true const ownsDebugUI = featureEnabled && !props.placementDebugState - const ownDebugState = usePlacementDebug(placement) + const ownDebugState = usePlacementDebug(ownsDebugUI ? placement : undefined) const { matched, notMatched, totalClusters, matchedCount, error } = props.placementDebugState ?? ownDebugState const { t } = useTranslation() From ee10572416967a334d57d0ad2522e7559be00cc8 Mon Sep 17 00:00:00 2001 From: fxiang1 Date: Wed, 29 Apr 2026 10:24:09 -0400 Subject: [PATCH 3/3] Fix unit tests Signed-off-by: fxiang1 --- frontend/src/wizards/Argo/ArgoWizard.test.tsx | 1 + frontend/src/wizards/Governance/Policy/policyWizard.test.tsx | 3 +++ 2 files changed, 4 insertions(+) diff --git a/frontend/src/wizards/Argo/ArgoWizard.test.tsx b/frontend/src/wizards/Argo/ArgoWizard.test.tsx index d01ab2ee5b5..3e0b811c816 100644 --- a/frontend/src/wizards/Argo/ArgoWizard.test.tsx +++ b/frontend/src/wizards/Argo/ArgoWizard.test.tsx @@ -113,6 +113,7 @@ describe('ArgoWizard tests', () => { test('should have danger alert', async () => { nockIgnoreOperatorCheck(true) + nockIgnorePlacementDebug() render( diff --git a/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx b/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx index 64657d2b452..a2826eef05d 100644 --- a/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx +++ b/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx @@ -17,6 +17,7 @@ import { waitForText } from '../../../lib/test-util' import { Policy } from '../../../resources' import { WizardSyncEditor } from '../../../routes/Governance/policies/CreatePolicy' import { isExistingTemplateName, PolicyWizard } from './PolicyWizard' +import { nockIgnorePlacementDebug } from '~/lib/nock-util' describe('ExistingTemplateName', () => { test('should return false for non-existing name', () => { @@ -128,6 +129,7 @@ function TestPolicyWizardOperatorPolicy() { describe('Policy wizard', () => { test('can show correct cluster sets dropdown', async () => { + nockIgnorePlacementDebug() const { container } = render() const nameTextbox = screen.getByRole('textbox', { name: /name/i }) @@ -296,6 +298,7 @@ describe('Policy wizard', () => { }) test('default tolerations are set when creating new placement', async () => { + nockIgnorePlacementDebug() render( } />) const nameTextbox = screen.getByRole('textbox', { name: /name/i })