From 1373b0456a1e186031df5bd677c09e31f337f368 Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Sun, 13 Jul 2025 18:36:27 +0530 Subject: [PATCH] Enhance Permission Handling in Data Quality Components - Integrated permission checks for creating and editing test cases in TestCaseFormV1 and BundleSuiteForm. - Updated state management to reflect permission-based conditions for enabling features like pipeline creation and scheduler options. - Refactored related components to ensure proper dependency handling in hooks. - Improved test coverage for permission-related functionalities across affected components. --- .../components/TestCaseFormV1.test.tsx | 19 +++ .../components/TestCaseFormV1.tsx | 67 +++++---- .../AddTestCaseList.component.tsx | 6 +- .../BundleSuiteForm/BundleSuiteForm.test.tsx | 15 +- .../BundleSuiteForm/BundleSuiteForm.tsx | 141 +++++++++--------- 5 files changed, 150 insertions(+), 98 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx index 26193cda0f2..07b0103ba14 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx @@ -137,6 +137,25 @@ jest.mock('../../../../context/LimitsProvider/useLimitsStore', () => ({ useLimitStore: () => ({ getResourceLimit: () => 100 }), })); +jest.mock('../../../../context/PermissionProvider/PermissionProvider', () => ({ + usePermissionProvider: () => ({ + getEntityPermissionByFqn: jest.fn().mockResolvedValue({ + EditAll: true, + EditTests: true, + }), + permissions: { + ingestionPipeline: { + Create: true, + EditAll: true, + }, + testCase: { + Create: true, + EditAll: true, + }, + }, + }), +})); + jest.mock('crypto-random-string-with-promisify-polyfill', () => jest.fn().mockReturnValue('ABC123') ); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx index 14b65b4bd4c..2bd531662b1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx @@ -147,7 +147,7 @@ const TestCaseFormV1: FC = ({ const [form] = useForm(); const { isAirflowAvailable } = useAirflowStatus(); const { getEntityPermissionByFqn, permissions } = usePermissionProvider(); - const { ingestionPipeline } = permissions; + const { ingestionPipeline, testCase } = permissions; const TEST_LEVEL_OPTIONS: SelectionOption[] = [ { @@ -457,19 +457,22 @@ const TestCaseFormV1: FC = ({ // HOOKS - Callbacks (grouped by functionality) // ============================================= // Pipeline-related callbacks - const checkExistingPipelines = useCallback(async (testSuiteFqn: string) => { - try { - const { paging } = await getIngestionPipelines({ - testSuite: testSuiteFqn, - pipelineType: [PipelineType.TestSuite], - arrQueryFields: ['id'], - limit: 0, - }); - setCanCreatePipeline(paging.total === 0); - } catch (error) { - setCanCreatePipeline(true); - } - }, []); + const checkExistingPipelines = useCallback( + async (testSuiteFqn: string) => { + try { + const { paging } = await getIngestionPipelines({ + testSuite: testSuiteFqn, + pipelineType: [PipelineType.TestSuite], + arrQueryFields: ['id'], + limit: 0, + }); + setCanCreatePipeline(paging.total === 0); + } catch (error) { + setCanCreatePipeline(true); + } + }, + [ingestionPipeline] + ); // Form interaction callbacks const handleCancel = useCallback(() => { @@ -647,8 +650,11 @@ const TestCaseFormV1: FC = ({ // Permission checking callback const checkTablePermissions = useCallback( async (tableFqn: string) => { - setIsCheckingPermissions(true); + if (testCase.Create) { + return Promise.resolve(); + } + setIsCheckingPermissions(true); try { const permissions = await getEntityPermissionByFqn( ResourceEntity.TABLE, @@ -656,6 +662,8 @@ const TestCaseFormV1: FC = ({ ); const canCreate = permissions.EditAll || permissions.EditTests; + setCanCreatePipeline(canCreate); + if (!canCreate) { // Return false to trigger validation error return Promise.reject( @@ -884,24 +892,31 @@ const TestCaseFormV1: FC = ({ // Check for existing pipelines when table is selected useEffect(() => { - if (selectedTableData?.testSuite?.fullyQualifiedName) { - // Table has test suite - check for existing pipelines - checkExistingPipelines(selectedTableData.testSuite.fullyQualifiedName); - } else if (testSuite?.fullyQualifiedName) { - // Using provided test suite - check for existing pipelines - checkExistingPipelines(testSuite.fullyQualifiedName); - } else if (selectedTable) { - // Table selected but no test suite - can create pipeline (test suite will be created) - setCanCreatePipeline(true); - } else { - // No table selected - hide scheduler + // Early return if user doesn't have permission to create pipelines or Early return if no table is selected + if (!ingestionPipeline.Create || !selectedTable) { setCanCreatePipeline(false); + + return; + } + + // Get the test suite FQN from either the table data or provided test suite + const testSuiteFqn = + selectedTableData?.testSuite?.fullyQualifiedName || + testSuite?.fullyQualifiedName; + + if (testSuiteFqn) { + // Check for existing pipelines if we have a test suite + checkExistingPipelines(testSuiteFqn); + } else { + // No test suite exists - can create pipeline (test suite will be created) + setCanCreatePipeline(true); } }, [ selectedTableData?.testSuite?.fullyQualifiedName, testSuite?.fullyQualifiedName, selectedTable, checkExistingPipelines, + ingestionPipeline, ]); // Initialize manual edit flag diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx index d56656a7d46..b1ff5ead48b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx @@ -100,7 +100,7 @@ export const AddTestCaseList = ({ setIsLoading(false); } }, - [selectedTest, filters, testCaseParams] + [selectedTest] ); const handleSubmit = async () => { @@ -123,7 +123,7 @@ export const AddTestCaseList = ({ }); } }, - [searchTerm, totalCount, items, isLoading, pageNumber, fetchTestCases] + [searchTerm, totalCount, items, isLoading] ); const handleCardClick = (details: TestCase) => { @@ -163,7 +163,7 @@ export const AddTestCaseList = ({ }; useEffect(() => { fetchTestCases({ searchText: searchTerm }); - }, [searchTerm, fetchTestCases]); + }, [searchTerm]); const renderList = useMemo(() => { if (!isLoading && isEmpty(items)) { diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.test.tsx index e81ada39ee8..2c06238fbb5 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.test.tsx @@ -97,6 +97,17 @@ jest.mock('../../../context/LimitsProvider/useLimitsStore', () => ({ }), })); +jest.mock('../../../context/PermissionProvider/PermissionProvider', () => ({ + usePermissionProvider: () => ({ + permissions: { + ingestionPipeline: { + Create: true, + EditAll: true, + }, + }, + }), +})); + jest.mock('../../../hooks/useApplicationStore', () => ({ useApplicationStore: jest.fn().mockReturnValue({ currentUser: { @@ -208,7 +219,9 @@ describe('BundleSuiteForm Component', () => { it('should render form in drawer mode', async () => { render(); - expect(await screen.findAllByText('label.create-entity')).toHaveLength(2); // One in header, one in card title + expect( + await screen.findByText('label.create-entity') + ).toBeInTheDocument(); // Should appear in card title expect(document.querySelector('.bundle-suite-form')).toBeInTheDocument(); expect(document.querySelector('.drawer-mode')).toBeInTheDocument(); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.tsx index afe13f90390..adab3781258 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/BundleSuiteForm/BundleSuiteForm.tsx @@ -34,6 +34,7 @@ import { MAX_NAME_LENGTH } from '../../../constants/constants'; import { DEFAULT_SCHEDULE_CRON_DAILY } from '../../../constants/Schedular.constants'; import { useAirflowStatus } from '../../../context/AirflowStatusProvider/AirflowStatusProvider'; import { useLimitStore } from '../../../context/LimitsProvider/useLimitsStore'; +import { usePermissionProvider } from '../../../context/PermissionProvider/PermissionProvider'; import { OwnerType } from '../../../enums/user.enum'; import { ConfigType, @@ -92,6 +93,8 @@ const BundleSuiteForm: React.FC = ({ const { config } = useLimitStore(); const { currentUser } = useApplicationStore(); const { isAirflowAvailable } = useAirflowStatus(); + const { permissions } = usePermissionProvider(); + const { ingestionPipeline } = permissions; const enableScheduler = Form.useWatch('enableScheduler', form); // ============================================= @@ -266,7 +269,7 @@ const BundleSuiteForm: React.FC = ({ testSuiteId: testSuite.id ?? '', }); - if (formData.enableScheduler) { + if (formData.enableScheduler && ingestionPipeline.Create) { await createAndDeployPipeline(testSuite, formData); } @@ -391,75 +394,77 @@ const BundleSuiteForm: React.FC = ({ {/* Scheduler with Toggle */} - -
- - - -
- - {t('label.create-entity', { - entity: t('label.pipeline'), - })} - - - {`${t('message.pipeline-entity-description', { - entity: t('label.bundle-suite'), - })} (${t('label.optional')})`} - -
-
- - {/* Scheduler form fields - only show when toggle is enabled */} - {enableScheduler && ( - <> - {generateFormFields(schedulerFormFields)} - - - + {ingestionPipeline.Create && ( + +
+ + - - {/* Debug Log and Raise on Error switches */} -
- - -
- - - - - {t('label.enable-debug-log')} - -
- - -
- - - - - {t('label.raise-on-error')} - -
- - +
+ + {t('label.create-entity', { + entity: t('label.pipeline'), + })} + + + {`${t('message.pipeline-entity-description', { + entity: t('label.bundle-suite'), + })} (${t('label.optional')})`} +
- - )} - +
+ + {/* Scheduler form fields - only show when toggle is enabled */} + {enableScheduler && ( + <> + {generateFormFields(schedulerFormFields)} + + + + + + {/* Debug Log and Raise on Error switches */} +
+ + +
+ + + + + {t('label.enable-debug-log')} + +
+ + +
+ + + + + {t('label.raise-on-error')} + +
+ + +
+ + )} + + )}
);