From 1044b965cc2c508180278199515f66c98bc54181 Mon Sep 17 00:00:00 2001 From: bteke Date: Sat, 21 Mar 2026 13:01:37 +0100 Subject: [PATCH 1/2] YARN-11943. Improve queue deletion flow. --- .../components/PropertyEditorTab.test.tsx | 39 +++++++++++++ .../components/PropertyEditorTab.tsx | 16 +----- .../components/PropertyPanel.test.tsx | 57 +++++++++++++++++++ .../components/PropertyPanel.tsx | 36 ++++++++++-- .../components/QueueCardContextMenu.tsx | 31 +++++++--- .../components/QueueCardNode.test.tsx | 41 +++++++++++++ .../components/QueueCardNode.tsx | 10 +++- .../QueueVisualizationContainer.tsx | 2 +- .../dialogs/DeleteQueueDialog.test.tsx | 46 ++++++--------- .../components/dialogs/DeleteQueueDialog.tsx | 53 +++++------------ .../webapp/src/stores/schedulerStore.test.ts | 35 ++++++++++++ .../src/stores/slices/stagedChangesSlice.ts | 9 +++ .../main/webapp/src/stores/slices/types.ts | 1 + 13 files changed, 277 insertions(+), 99 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.test.tsx b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.test.tsx index cb239d1680370..e1f85519e7e72 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.test.tsx +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.test.tsx @@ -22,11 +22,28 @@ import userEvent from '@testing-library/user-event'; import { vi } from 'vitest'; import { PropertyEditorTab } from './PropertyEditorTab'; import { usePropertyEditor } from '~/features/property-editor/hooks/usePropertyEditor'; +import { useSchedulerStore } from '~/stores/schedulerStore'; import type { QueueInfo, PropertyDescriptor } from '~/types'; // Mock the hooks vi.mock('~/features/property-editor/hooks/usePropertyEditor'); +// Mock the scheduler store +vi.mock('~/stores/schedulerStore', () => ({ + useSchedulerStore: vi.fn((selector: any) => { + const state = { + getGlobalPropertyValue: vi.fn().mockReturnValue({ value: '' }), + getQueuePropertyValue: vi.fn().mockReturnValue({ value: '' }), + stagedChanges: [], + configData: new Map(), + schedulerData: null, + hasPendingDeletion: vi.fn().mockReturnValue(false), + revertQueueDeletion: vi.fn(), + }; + return selector ? selector(state) : state; + }), +})); + // Mock toast vi.mock('sonner', () => ({ toast: { @@ -161,6 +178,28 @@ describe('PropertyEditorTab', () => { expect(within(capacityTrigger).getByText('1')).toBeInTheDocument(); }); + describe('pending deletion state', () => { + it('should not show deletion banner in PropertyEditorTab (banner is in PropertyPanel)', () => { + vi.mocked(useSchedulerStore).mockImplementation((selector: any) => { + const state = { + getGlobalPropertyValue: vi.fn().mockReturnValue({ value: '' }), + getQueuePropertyValue: vi.fn().mockReturnValue({ value: '' }), + stagedChanges: [], + configData: new Map(), + schedulerData: null, + hasPendingDeletion: vi.fn().mockReturnValue(true), + revertQueueDeletion: vi.fn(), + }; + return selector ? selector(state) : state; + }); + + render(); + + expect(screen.queryByText(/marked for deletion/i)).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /undo delete/i })).not.toBeInTheDocument(); + }); + }); + it('renders template configuration button when controls allow management', async () => { const templateProperty = { name: 'auto-queue-creation-v2.enabled', diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.tsx b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.tsx index e99db039d45a1..150f07438d4c8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.tsx +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyEditorTab.tsx @@ -26,8 +26,6 @@ import { AccordionItem, AccordionTrigger, } from '~/components/ui/accordion'; -import { Alert, AlertDescription } from '~/components/ui/alert'; -import { AlertTriangle } from 'lucide-react'; import { usePropertyEditor } from '~/features/property-editor/hooks/usePropertyEditor'; import { PropertyFormField } from './PropertyFormField'; import type { QueueInfo } from '~/types'; @@ -364,18 +362,6 @@ export const PropertyEditorTab = ({ return (
- {/* Warning banner for queues pending deletion */} - {isPendingDeletion && ( -
- - - - This queue is pending deletion and will be removed when changes are applied. - - -
- )} - {/* Loading State */} {isFormInitializing && (
@@ -436,7 +422,7 @@ export const PropertyEditorTab = ({ property={prop} control={control} stagedStatus={getStagedStatus(prop.originalName || prop.name)} - isEnabled={propertyState?.enabled ?? true} + isEnabled={(propertyState?.enabled ?? true) && !isPendingDeletion} onBlur={handleFieldBlur} errors={getFieldErrors(prop.formFieldName || prop.name)} warnings={getFieldWarnings(prop.formFieldName || prop.name)} diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx index c3bd2bb5f4c56..22c4f90af00a4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx @@ -147,6 +147,8 @@ function getBaseStoreState(): Partial { stagedChanges: [], configData: new Map(), schedulerData: null, + hasPendingDeletion: vi.fn().mockReturnValue(false), + revertQueueDeletion: vi.fn(), shouldOpenTemplateConfig: false, requestTemplateConfigOpen: () => { storeState = { ...storeState, shouldOpenTemplateConfig: true }; @@ -542,4 +544,59 @@ describe('PropertyPanel', () => { // Would need to set isSubmitting state through the PropertyEditorTab callbacks // This is tested through integration tests }); + + describe('pending deletion state', () => { + it('should hide Stage Changes and Reset buttons when queue is pending deletion', async () => { + const user = userEvent.setup(); + setStoreState({ + selectedQueuePath: 'root.default', + isPropertyPanelOpen: true, + stagedChanges: [ + { + id: 'removal-1', + queuePath: 'root.default', + property: '__QUEUE_MARKER__', + type: 'remove', + oldValue: 'exists', + newValue: undefined, + }, + ] as any, + revertQueueDeletion: vi.fn(), + }); + mockGetQueueByPath.mockReturnValue(mockQueue); + + render(); + + // Switch to settings tab to verify buttons are hidden + const settingsTab = screen.getByRole('tab', { name: /settings/i }); + await user.click(settingsTab); + + expect(screen.queryByText('Stage Changes')).not.toBeInTheDocument(); + expect(screen.queryByText('Reset')).not.toBeInTheDocument(); + }); + + it('should show deletion badge and undo button when queue is pending deletion', () => { + setStoreState({ + selectedQueuePath: 'root.default', + isPropertyPanelOpen: true, + stagedChanges: [ + { + id: 'removal-1', + queuePath: 'root.default', + property: '__QUEUE_MARKER__', + type: 'remove', + oldValue: 'exists', + newValue: undefined, + }, + ] as any, + revertQueueDeletion: vi.fn(), + }); + mockGetQueueByPath.mockReturnValue(mockQueue); + + render(); + + expect(screen.getByText(/marked for deletion/i)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /undo/i })).toBeInTheDocument(); + }); + }); }); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.tsx b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.tsx index 7a78f72e768d2..daa84a4e0d5fd 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.tsx +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.tsx @@ -18,7 +18,7 @@ import React, { useReducer, useState, useEffect, useRef } from 'react'; -import { Save, RotateCcw, GitBranch, Info, Settings, Edit, AlertTriangle } from 'lucide-react'; +import { Save, RotateCcw, GitBranch, Info, Settings, Edit, AlertTriangle, Undo2, Trash2 } from 'lucide-react'; import { useShallow } from 'zustand/react/shallow'; import { useSchedulerStore } from '~/stores/schedulerStore'; import { Sheet, SheetContent, SheetHeader, SheetTitle } from '~/components/ui/sheet'; @@ -110,6 +110,7 @@ export const PropertyPanel: React.FC = () => { const selectQueue = useSchedulerStore((s) => s.selectQueue); const clearTemplateConfigRequest = useSchedulerStore((s) => s.clearTemplateConfigRequest); const getQueuePropertyValue = useSchedulerStore((s) => s.getQueuePropertyValue); + const revertQueueDeletion = useSchedulerStore((s) => s.revertQueueDeletion); const [formState, dispatch] = useReducer(formReducer, INITIAL_FORM_STATE); const { hasChanges, isFormDirty } = formState; @@ -282,6 +283,10 @@ export const PropertyPanel: React.FC = () => { } }, [isPropertyPanelOpen]); + const isPendingDeletion = selectedQueuePath + ? stagedChanges.some((c) => c.queuePath === selectedQueuePath && c.type === 'remove') + : false; + const queuePath = selectedQueue?.queuePath; const queueIssues = !queuePath ? {} : (validationState[queuePath] ?? {}); @@ -308,7 +313,7 @@ export const PropertyPanel: React.FC = () => { // Keyboard shortcuts - only active when panel is open and on settings tab useKeyboardShortcuts( - isPanelVisible && tabValue === 'settings' + isPanelVisible && tabValue === 'settings' && !isPendingDeletion ? [ { key: 's', @@ -366,13 +371,34 @@ export const PropertyPanel: React.FC = () => { issues={issueList} onIssueSelect={handleIssueSelect} /> - {isFormDirty && ( + {isPendingDeletion && ( + <> + + + Marked for deletion + + + + )} + {!isPendingDeletion && isFormDirty && ( Unsaved )} - {!isFormDirty && hasChanges && ( + {!isPendingDeletion && !isFormDirty && hasChanges && ( Staged @@ -444,7 +470,7 @@ export const PropertyPanel: React.FC = () => { {/* Fixed Apply/Reset buttons - show on Settings tab */} - {tabValue === 'settings' && ( + {tabValue === 'settings' && !isPendingDeletion && (
{canDelete && !isRoot && ( - )} diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/schedulerStore.test.ts b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/schedulerStore.test.ts index dac74653707ad..82192f353fd2c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/schedulerStore.test.ts +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/schedulerStore.test.ts @@ -796,6 +796,41 @@ describe('schedulerStore', () => { }); }); + describe('revertQueueDeletion', () => { + it('should revert a pending queue deletion', () => { + const store = createTestStore(); + store.getState().stageQueueRemoval('root.test'); + + expect(store.getState().hasPendingDeletion('root.test')).toBe(true); + + store.getState().revertQueueDeletion('root.test'); + + expect(store.getState().hasPendingDeletion('root.test')).toBe(false); + expect(store.getState().stagedChanges).toHaveLength(0); + }); + + it('should not affect other staged changes', () => { + const store = createTestStore(); + store.getState().stageQueueChange('root.other', 'capacity', '50'); + store.getState().stageQueueRemoval('root.test'); + + store.getState().revertQueueDeletion('root.test'); + + expect(store.getState().hasPendingDeletion('root.test')).toBe(false); + expect(store.getState().stagedChanges).toHaveLength(1); + expect(store.getState().stagedChanges[0].queuePath).toBe('root.other'); + }); + + it('should be a no-op if queue has no pending deletion', () => { + const store = createTestStore(); + store.getState().stageQueueChange('root.test', 'capacity', '50'); + + store.getState().revertQueueDeletion('root.test'); + + expect(store.getState().stagedChanges).toHaveLength(1); + }); + }); + describe('clearAllChanges', () => { it('should clear all staged changes', () => { const store = createTestStore(); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/stagedChangesSlice.ts b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/stagedChangesSlice.ts index d4b8fe029128d..cd9b02bccc7c1 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/stagedChangesSlice.ts +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/stagedChangesSlice.ts @@ -542,6 +542,15 @@ export const createStagedChangesSlice: StateCreator< return get().stagedChanges.some((c) => c.queuePath === queuePath && c.type === 'remove'); }, + revertQueueDeletion: (queuePath) => { + const removalChange = get().stagedChanges.find( + (c) => c.queuePath === queuePath && c.type === 'remove', + ); + if (removalChange) { + get().revertChange(removalChange.id); + } + }, + getStagedChangeById: (changeId) => { return get().stagedChanges.find((c) => c.id === changeId); }, diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/types.ts b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/types.ts index e93656415179a..02fb6b7900115 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/types.ts +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/stores/slices/types.ts @@ -94,6 +94,7 @@ export interface StagedChangesSlice { ) => void; applyChanges: () => Promise; revertChange: (changeId: string) => void; + revertQueueDeletion: (queuePath: string) => void; clearAllChanges: () => void; clearQueueChanges: (queuePath: string) => void; hasUnsavedChanges: () => boolean; From f2593f70406d1714418f12f7fd3c92a10fc9e968 Mon Sep 17 00:00:00 2001 From: bteke Date: Sun, 22 Mar 2026 01:00:01 +0100 Subject: [PATCH 2/2] Fix constant in tests. --- .../property-editor/components/PropertyPanel.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx index 22c4f90af00a4..763e2ebe4ad5a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-capacity-scheduler-ui/src/main/webapp/src/features/property-editor/components/PropertyPanel.test.tsx @@ -24,6 +24,7 @@ import userEvent from '@testing-library/user-event'; import { useSchedulerStore } from '~/stores/schedulerStore'; import type { SchedulerStore } from '~/stores/schedulerStore'; import type { QueueInfo } from '~/types'; +import { SPECIAL_VALUES } from '~/types/constants/special-values'; import { toast } from 'sonner'; // Test helper @@ -555,7 +556,7 @@ describe('PropertyPanel', () => { { id: 'removal-1', queuePath: 'root.default', - property: '__QUEUE_MARKER__', + property: SPECIAL_VALUES.QUEUE_MARKER, type: 'remove', oldValue: 'exists', newValue: undefined, @@ -583,7 +584,7 @@ describe('PropertyPanel', () => { { id: 'removal-1', queuePath: 'root.default', - property: '__QUEUE_MARKER__', + property: SPECIAL_VALUES.QUEUE_MARKER, type: 'remove', oldValue: 'exists', newValue: undefined,