Fixes #35536: Add and edit custom metrics in one drawer

.github/playwright/impact-map.generated.json

@@ -143,6 +143,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityDashboard.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityPermissions.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerDateFilter.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerSqlQuery.spec.ts",
         "playwright/e2e/Features/DataQuality/Profiler.spec.ts",
@@ -749,6 +750,7 @@
         "playwright/e2e/Features/AppMode/AppModeUrlHandling.spec.ts",
         "playwright/e2e/Features/ApplicationsSettingsModal.spec.ts",
         "playwright/e2e/Features/CustomMetric.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerDateFilter.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerSqlQuery.spec.ts",
         "playwright/e2e/Features/DataQuality/RunTestCase.spec.ts",
@@ -802,6 +804,7 @@
         "playwright/e2e/Features/DataQuality/AddTestCaseNewFlow.spec.ts",
         "playwright/e2e/Features/DataQuality/BundleSuiteBulkOperations.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQuality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerDateFilter.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerSqlQuery.spec.ts",
         "playwright/e2e/Features/DataQuality/Profiler.spec.ts",
@@ -1685,6 +1688,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityListAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityPermissions.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterOwnerChange.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts",
@@ -3347,6 +3351,7 @@
         "openmetadata-ui/src/main/resources/ui/playwright/utils/codeEditor.ts"
       ],
       "specs": [
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/Profiler.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
         "playwright/e2e/Features/Permission.spec.ts",
@@ -3448,6 +3453,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityListAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityPermissions.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterOwnerChange.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts",
@@ -3880,6 +3886,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityDashboard.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityDashboardAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
         "playwright/e2e/Features/IncidentManager.spec.ts",
         "playwright/e2e/Features/TestSuiteMultiPipeline.spec.ts",
@@ -5120,6 +5127,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityDashboard.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityPermissions.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/Profiler.spec.ts",
         "playwright/e2e/Features/DataQuality/RunTestCase.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
@@ -6384,6 +6392,7 @@
         "playwright/e2e/Features/DataQuality/ColumnLevelTests.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQuality.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
         "playwright/e2e/Pages/DataContracts.spec.ts",
         "playwright/e2e/VersionPages/TestCaseVersionPage.spec.ts"
@@ -6524,6 +6533,7 @@
         "openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/RunDetailsCard/RunDetailsCard.tsx"
       ],
       "specs": [
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/TestCaseDetailsPage.spec.ts"
       ]
     },
@@ -6798,6 +6808,7 @@
         "openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/CodeEditor.tsx"
       ],
       "specs": [
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
         "playwright/e2e/Features/QueryEntity.spec.ts",
         "playwright/e2e/Pages/DataContracts.spec.ts"
@@ -6808,6 +6819,7 @@
         "openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaEditor/SchemaEditor.tsx"
       ],
       "specs": [
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/TableLevelTests.spec.ts",
         "playwright/e2e/Features/QueryEntity.spec.ts",
         "playwright/e2e/Pages/DataContracts.spec.ts"
@@ -6979,7 +6991,9 @@
         "openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityExportModalProvider/EntityExportModal.component.tsx"
       ],
       "specs": [
-        "playwright/e2e/Features/LineageExportPNGSnapshot.spec.ts"
+        "playwright/e2e/Features/LineageExportPNGSnapshot.spec.ts",
+        "playwright/e2e/Pages/TestSuite.spec.ts",
+        "playwright/e2e/Pages/TestSuiteDetailsPage.spec.ts"
       ]
     },
     {
@@ -11320,6 +11334,14 @@
         "playwright/e2e/Features/DataQuality/DataQualityDashboard.spec.ts"
       ]
     },
+    {
+      "sources": [
+        "openmetadata-ui/src/main/resources/ui/src/components/observability/data-quality/ThresholdPreview/ThresholdPreview.tsx"
+      ],
+      "specs": [
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts"
+      ]
+    },
     {
       "sources": [
         "openmetadata-ui/src/main/resources/ui/src/components/platform/ai-shell/AINotFoundPage/AINotFoundPage.tsx"
@@ -11876,6 +11898,7 @@
         "playwright/e2e/Features/DataQuality/DataQualityListAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/DataQualityPermissions.spec.ts",
         "playwright/e2e/Features/DataQuality/Dimensionality.spec.ts",
+        "playwright/e2e/Features/DataQuality/FailureThreshold.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterOwnerChange.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterReindex.spec.ts",
         "playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts",
@@ -12751,6 +12774,16 @@
         "playwright/e2e/Pages/SearchSettings.spec.ts"
       ]
     },
+    {
+      "sources": [
+        "openmetadata-ui/src/main/resources/ui/src/pages/ForgotPassword/ForgotPassword.component.tsx"
+      ],
+      "specs": [
+        "playwright/e2e/Features/LineageExportPNGSnapshot.spec.ts",
+        "playwright/e2e/Pages/TestSuite.spec.ts",
+        "playwright/e2e/Pages/TestSuiteDetailsPage.spec.ts"
+      ]
+    },
     {
       "sources": [
         "openmetadata-ui/src/main/resources/ui/src/pages/GlobalSettingPage/GlobalSettingCategory/GlobalSettingCategoryPage.tsx"
@@ -12989,6 +13022,16 @@
         "playwright/e2e/Pages/ProfilerConfigurationPage.spec.ts"
       ]
     },
+    {
+      "sources": [
+        "openmetadata-ui/src/main/resources/ui/src/pages/ResetPassword/ResetPassword.component.tsx"
+      ],
+      "specs": [
+        "playwright/e2e/Features/LineageExportPNGSnapshot.spec.ts",
+        "playwright/e2e/Pages/TestSuite.spec.ts",
+        "playwright/e2e/Pages/TestSuiteDetailsPage.spec.ts"
+      ]
+    },
     {
       "sources": [
         "openmetadata-ui/src/main/resources/ui/src/pages/RolesPage/AddAttributeModal/AddAttributeModal.tsx"

.github/playwright/impact-map.json

@@ -736,7 +736,7 @@
       "_comment": "Metric page, custom-metric page, bulk-edit grid, and the CSV import/export machinery all funnel through this cluster. MetricBulkImportExportEdit + BulkImport + ClassificationImportExport have been ejecting PRs from the merge queue because none of them named these paths — PR CI skipped them entirely and the queue was the first place they ran. The two playwright helpers are included so a wait/selector fix in importUtils.ts also routes back to the specs that consume it.",
       "sources": [
         "openmetadata-ui/src/main/resources/ui/src/pages/MetricsPage/**",
-        "openmetadata-ui/src/main/resources/ui/src/pages/AddCustomMetricPage/**",
+        "openmetadata-ui/src/main/resources/ui/src/components/DataQuality/CustomMetricForm/**",
         "openmetadata-ui/src/main/resources/ui/src/components/Metric/**",
         "openmetadata-ui/src/main/resources/ui/src/components/BulkEditEntity/**",
         "openmetadata-ui/src/main/resources/ui/src/rest/metricsAPI.ts",

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/CustomMetric.spec.ts

@@ -17,6 +17,8 @@ import { getApiContext, redirectToHomePage, uuid } from '../../utils/common';
 import {
   createCustomMetric,
   deleteCustomMetric,
+  editCustomMetric,
+  openCustomMetricDrawer,
 } from '../../utils/customMetric';
 import { waitForAllLoadersToDisappear } from '../../utils/entity';
 import { disableAiAppMode, stubUserPreferencesAppMode } from '../Utils/appMode';
@@ -59,6 +61,16 @@ test('Table custom metric', async ({ page }) => {
     });
   });
 
+  await test.step('Edit', async () => {
+    await editCustomMetric({
+      page,
+      metric: {
+        ...TABLE_CUSTOM_METRIC,
+        expression: `SELECT COUNT(*) FROM ${table.entity.name}`,
+      },
+    });
+  });
+
   await test.step('Delete', async () => {
     await deleteCustomMetric({
       page,
@@ -99,6 +111,17 @@ test('Column custom metric', async ({ page }) => {
     });
   });
 
+  await test.step('Edit', async () => {
+    await editCustomMetric({
+      page,
+      isColumnMetric: true,
+      metric: {
+        ...COLUMN_CUSTOM_METRIC,
+        expression: `SELECT COUNT(*) FROM ${table.entity.name}`,
+      },
+    });
+  });
+
   await test.step('Delete', async () => {
     await deleteCustomMetric({
       page,
@@ -125,9 +148,13 @@ test('Large column Select preserves keyboard navigation and fully reveals the fo
 
   try {
     await table.create(apiContext);
-    await page.goto(
-      `/add-custom-metric/column/${table.entityResponseData.fullyQualifiedName}`
+    const profilerResponse = page.waitForResponse(
+      `/api/v1/tables/${table.entityResponseData?.['fullyQualifiedName']}/tableProfile/latest?includeColumnProfile=false`
     );
+    await table.visitEntityPage(page);
+    await page.click('[data-testid="profiler"]');
+    await profilerResponse;
+    await openCustomMetricDrawer(page, true);
     const trigger = page
       .getByTestId('custom-metric-column')
       .getByRole('button');

openmetadata-ui/src/main/resources/ui/playwright/utils/customMetric.ts

@@ -16,7 +16,7 @@ import {
   NAME_MAX_LENGTH_VALIDATION_ERROR,
   NAME_VALIDATION_ERROR,
 } from '../constant/common';
-import { clickCodeEditor } from './codeEditor';
+import { clickCodeEditor, fillCodeEditor } from './codeEditor';
 import { toastNotification } from './common';
 import {
   ObservabilityFeature,
@@ -78,46 +78,54 @@ const validateForm = async (page: Page, isColumnMetric = false) => {
   await page.getByTestId('custom-metric-name').clear();
 };
 
-export const createCustomMetric = async ({
-  page,
-  isColumnMetric = false,
-  metric,
-}: CustomMetricDetails) => {
+export const openCustomMetricDrawer = async (
+  page: Page,
+  isColumnMetric = false
+) => {
   await page
     .getByRole('tab', {
       name: isColumnMetric ? 'Column Profile' : 'Table Profile',
     })
     .click();
-  await page.locator('[data-testid="profiler-add-table-test-btn"]').click();
+  await page.getByTestId('profiler-add-table-test-btn').click();
   await selectAddObservabilityFeature(page, ObservabilityFeature.CUSTOM_METRIC);
 
-  const customMetricResponse = page.waitForResponse(
-    '/api/v1/tables/name/*?fields=customMetrics%2Ccolumns&include=all*'
-  );
+  const drawer = page.getByTestId('custom-metric-form-drawer');
+
+  await expect(drawer).toBeVisible();
+
+  return drawer;
+};
+
+export const createCustomMetric = async ({
+  page,
+  isColumnMetric = false,
+  metric,
+}: CustomMetricDetails) => {
+  const drawer = await openCustomMetricDrawer(page, isColumnMetric);
 
-  // validate redirection and cancel button
-  await expect(page.locator('[data-testid="heading"]')).toBeVisible();
+  // the drawer keeps the profile preview beside the form
   await expect(
-    page
-      .locator(
-        `[data-testid="${
-          isColumnMetric
-            ? 'profiler-tab-container'
-            : 'table-profiler-chart-container'
-        }"]`
-      )
-      .filter({ visible: true })
-  ).not.toHaveCount(0);
-
-  await page.locator('[data-testid="cancel-button"]').click();
-  await customMetricResponse;
+    drawer.getByText(
+      isColumnMetric ? 'Add Column metric' : 'Add Table metric',
+      { exact: true }
+    )
+  ).toBeVisible();
+  await expect(
+    drawer.getByTestId(
+      isColumnMetric
+        ? 'profiler-tab-container'
+        : 'table-profiler-chart-container'
+    )
+  ).toBeVisible();
+
+  await drawer.getByTestId('cancel-button').click();
 
+  await expect(drawer).toBeHidden();
   await expect(page).toHaveURL(/profiler/);
 
-  // Click on create custom metric button
-  await page.click('[data-testid="profiler-add-table-test-btn"]');
-  await page.getByRole('menuitemradio', { name: 'Custom metric' }).click();
-  await page.click('[data-testid="submit-button"]');
+  await openCustomMetricDrawer(page, isColumnMetric);
+  await drawer.getByTestId('submit-button').click();
 
   await validateForm(page, isColumnMetric);
 
@@ -136,20 +144,61 @@ export const createCustomMetric = async ({
   const createMetricResponse = page.waitForResponse(
     '/api/v1/tables/*/customMetric'
   );
-  await page.click('[data-testid="submit-button"]');
+  await drawer.getByTestId('submit-button').click();
   await createMetricResponse;
 
   await toastNotification(
     page,
     new RegExp(`${metric.name} created successfully.`)
   );
 
-  // verify the created custom metric
-  await expect(page).toHaveURL(/profiler/);
-
+  await expect(drawer).toBeHidden();
   await expect(page.getByTestId(`${metric.name}-title`)).toBeVisible();
 };
 
+export const editCustomMetric = async ({
+  page,
+  metric,
+}: CustomMetricDetails) => {
+  await page.getByTestId(`${metric.name}-title`).scrollIntoViewIfNeeded();
+  await page.getByTestId(`${metric.name}-custom-metrics-menu`).click();
+  await page.getByRole('menuitemradio', { name: 'Edit' }).click();
+
+  const drawer = page.getByTestId('custom-metric-form-drawer');
+
+  await expect(drawer).toBeVisible();
+  await expect(drawer.getByTestId('custom-metric-name')).toHaveValue(
+    metric.name
+  );
+  await expect(drawer.getByTestId('custom-metric-name')).toBeDisabled();
+
+  await fillCodeEditor(page, drawer, metric.expression);
+
+  const updateMetricResponse = page.waitForResponse(
+    (response) =>
+      response.url().includes('/customMetric') &&
+      response.request().method() === 'PUT'
+  );
+  await drawer.getByTestId('submit-button').click();
+  const response = await updateMetricResponse;
+  const { customMetrics = [], columns = [] } = await response.json();
+  const savedMetric = [
+    ...customMetrics,
+    ...columns.flatMap(
+      (column: { customMetrics?: unknown[] }) => column.customMetrics ?? []
+    ),
+  ].find((saved: { name: string }) => saved.name === metric.name);
+
+  expect(savedMetric.expression).toBe(metric.expression);
+
+  await toastNotification(
+    page,
+    new RegExp(`${metric.name} updated successfully.`)
+  );
+
+  await expect(drawer).toBeHidden();
+};
+
 export const deleteCustomMetric = async ({
   page,
   metric,