fix: #19621 Introduce "clear sample" in entity config to have an explicit null (#19859)

* fix: #19621 Introduce "clear sample" in entity config to have an explicit `null` * added playwright test * added clear button * fixed playwright failure * addressing review comment
2025-11-03 20:19:31 +00:00 · 2025-03-12 09:29:10 +05:30 · 2025-03-12 09:29:10 +05:30 · a5eb90f797
commit a5eb90f797
parent 1a18c7d7f8
6 changed files with 217 additions and 102 deletions
--- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataQualityAndProfiler.spec.ts
+++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataQualityAndProfiler.spec.ts
@ -505,78 +505,137 @@ test(
    await page.reload();
    await page.waitForLoadState('networkidle');

-    await page.click('[data-testid="profiler-setting-btn"]');
-    await page.waitForSelector('.ant-modal-body');
-    await page.locator('[data-testid="slider-input"]').clear();
-    await page
-      .locator('[data-testid="slider-input"]')
-      .fill(profilerSetting.profileSample);
+    await test.step('Update profiler setting', async () => {
+      await page.click('[data-testid="profiler-setting-btn"]');
+      await page.waitForSelector('.ant-modal-body');

-    await page.locator('[data-testid="sample-data-count-input"]').clear();
-    await page
-      .locator('[data-testid="sample-data-count-input"]')
-      .fill(profilerSetting.sampleDataCount);
-    await page.locator('[data-testid="exclude-column-select"]').click();
-    await page.keyboard.type(`${profilerSetting.excludeColumns}`);
-    await page.keyboard.press('Enter');
-    await page.locator('.CodeMirror-scroll').click();
-    await page.keyboard.type(profilerSetting.profileQuery);
+      await page.locator('[data-testid="slider-input"]').clear();
+      await page
+        .locator('[data-testid="slider-input"]')
+        .fill(profilerSetting.profileSample);

-    await page.locator('[data-testid="include-column-select"]').click();
-    await page
-      .locator('.ant-select-dropdown')
-      .locator(
-        `[title="${profilerSetting.includeColumns}"]:not(.ant-select-dropdown-hidden)`
-      )
-      .last()
-      .click();
-    await page.locator('[data-testid="enable-partition-switch"]').click();
-    await page.locator('[data-testid="interval-type"]').click();
-    await page
-      .locator('.ant-select-dropdown')
-      .locator(
-        `[title="${profilerSetting.partitionIntervalType}"]:not(.ant-select-dropdown-hidden)`
-      )
-      .click();
+      await page.locator('[data-testid="sample-data-count-input"]').clear();
+      await page
+        .locator('[data-testid="sample-data-count-input"]')
+        .fill(profilerSetting.sampleDataCount);
+      await page.locator('[data-testid="exclude-column-select"]').click();
+      await page.keyboard.type(`${profilerSetting.excludeColumns}`);
+      await page.keyboard.press('Enter');
+      await page.locator('.CodeMirror-scroll').click();
+      await page.keyboard.type(profilerSetting.profileQuery);

-    await page.locator('#includeColumnsProfiler_partitionColumnName').click();
-    await page
-      .locator('.ant-select-dropdown')
-      .locator(
-        `[title="${profilerSetting.partitionColumnName}"]:not(.ant-select-dropdown-hidden)`
-      )
-      .last()
-      .click();
-    await page
-      .locator('[data-testid="partition-value"]')
-      .fill(profilerSetting.partitionValues);
+      await page.locator('[data-testid="include-column-select"]').click();
+      await page
+        .locator('.ant-select-dropdown')
+        .locator(
+          `[title="${profilerSetting.includeColumns}"]:not(.ant-select-dropdown-hidden)`
+        )
+        .last()
+        .click();
+      await page.locator('[data-testid="enable-partition-switch"]').click();
+      await page.locator('[data-testid="interval-type"]').click();
+      await page
+        .locator('.ant-select-dropdown')
+        .locator(
+          `[title="${profilerSetting.partitionIntervalType}"]:not(.ant-select-dropdown-hidden)`
+        )
+        .click();

-    const updateTableProfilerConfigResponse = page.waitForResponse(
-      (response) =>
-        response.url().includes('/api/v1/tables/') &&
-        response.url().includes('/tableProfilerConfig') &&
-        response.request().method() === 'PUT'
-    );
-    await page.getByRole('button', { name: 'Save' }).click();
-    const updateResponse = await updateTableProfilerConfigResponse;
-    const requestBody = await updateResponse.request().postData();
+      await page.locator('#includeColumnsProfiler_partitionColumnName').click();
+      await page
+        .locator('.ant-select-dropdown')
+        .locator(
+          `[title="${profilerSetting.partitionColumnName}"]:not(.ant-select-dropdown-hidden)`
+        )
+        .last()
+        .click();
+      await page
+        .locator('[data-testid="partition-value"]')
+        .fill(profilerSetting.partitionValues);

-    expect(requestBody).toEqual(
-      JSON.stringify({
-        excludeColumns: [table1.entity?.columns[0].name],
-        profileQuery: 'select * from table',
-        profileSample: 60,
-        profileSampleType: 'PERCENTAGE',
-        includeColumns: [{ columnName: table1.entity?.columns[1].name }],
-        partitioning: {
-          partitionColumnName: table1.entity?.columns[2].name,
-          partitionIntervalType: 'COLUMN-VALUE',
-          partitionValues: ['test'],
-          enablePartitioning: true,
-        },
-        sampleDataCount: 100,
-      })
-    );
+      const updateTableProfilerConfigResponse = page.waitForResponse(
+        (response) =>
+          response.url().includes('/api/v1/tables/') &&
+          response.url().includes('/tableProfilerConfig') &&
+          response.request().method() === 'PUT'
+      );
+      await page.getByRole('button', { name: 'Save' }).click();
+      const updateResponse = await updateTableProfilerConfigResponse;
+      const requestBody = await updateResponse.request().postData();
+
+      expect(requestBody).toEqual(
+        JSON.stringify({
+          excludeColumns: [table1.entity?.columns[0].name],
+          profileQuery: 'select * from table',
+          profileSample: 60,
+          profileSampleType: 'PERCENTAGE',
+          includeColumns: [{ columnName: table1.entity?.columns[1].name }],
+          partitioning: {
+            partitionColumnName: table1.entity?.columns[2].name,
+            partitionIntervalType: 'COLUMN-VALUE',
+            partitionValues: ['test'],
+            enablePartitioning: true,
+          },
+          sampleDataCount: 100,
+        })
+      );
+    });
+
+    await test.step('Reset profile sample type', async () => {
+      await page.click('[data-testid="profiler-setting-btn"]');
+      await page.waitForSelector('.ant-modal-body');
+
+      await expect(
+        page.locator('[data-testid="profile-sample"]')
+      ).toBeVisible();
+
+      await page.getByTestId('clear-slider-input').click();
+
+      await expect(page.locator('[data-testid="slider-input"]')).toBeEmpty();
+
+      const updateTableProfilerConfigResponse = page.waitForResponse(
+        (response) =>
+          response.url().includes('/api/v1/tables/') &&
+          response.url().includes('/tableProfilerConfig') &&
+          response.request().method() === 'PUT'
+      );
+      await page.getByRole('button', { name: 'Save' }).click();
+      const updateResponse = await updateTableProfilerConfigResponse;
+      const requestBody = await updateResponse.request().postData();
+
+      expect(requestBody).toEqual(
+        JSON.stringify({
+          excludeColumns: [table1.entity?.columns[0].name],
+          profileQuery: 'select * from table',
+          profileSample: null,
+          profileSampleType: 'PERCENTAGE',
+          includeColumns: [{ columnName: table1.entity?.columns[1].name }],
+          partitioning: {
+            partitionColumnName: table1.entity?.columns[2].name,
+            partitionIntervalType: 'COLUMN-VALUE',
+            partitionValues: ['test'],
+            enablePartitioning: true,
+          },
+          sampleDataCount: 100,
+        })
+      );
+
+      await page.waitForSelector('.ant-modal-body', {
+        state: 'detached',
+      });
+
+      // Validate the profiler setting is updated
+      await page.click('[data-testid="profiler-setting-btn"]');
+      await page.waitForSelector('.ant-modal-body');
+
+      await expect(
+        page.locator('[data-testid="profile-sample"]')
+      ).toBeVisible();
+      await expect(page.locator('[data-testid="slider-input"]')).toBeEmpty();
+      await expect(
+        page.getByTestId('profile-sample').locator('div')
+      ).toBeVisible();
+    });
  }
 );

--- a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ProfilerSettingsModal/ProfilerSettingsModal.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ProfilerSettingsModal/ProfilerSettingsModal.tsx
@ -175,24 +175,23 @@ const ProfilerSettingsModal: React.FC<ProfilerSettingsModalProps> = ({
      sqlQuery: profileQuery ?? '',
      profileSample: profileSample,
      excludeCol: excludeColumns ?? [],
-      selectedProfileSampleType:
-        profileSampleType ?? ProfileSampleType.Percentage,
+      selectedProfileSampleType: profileSampleType,
      sampleDataCount,
    });
    form.setFieldsValue({
      sampleDataCount: sampleDataCount ?? initialState.sampleDataCount,
    });

-    const profileSampleTypeCheck =
-      profileSampleType === ProfileSampleType.Percentage;
    form.setFieldsValue({
      profileSampleType,
-      profileSamplePercentage: profileSampleTypeCheck
-        ? profileSample ?? 100
-        : 100,
-      profileSampleRows: !profileSampleTypeCheck
-        ? profileSample ?? 100
-        : undefined,
+      profileSamplePercentage:
+        profileSample && profileSampleType === ProfileSampleType.Percentage
+          ? profileSample
+          : undefined,
+      profileSampleRows:
+        profileSample && profileSampleType === ProfileSampleType.Rows
+          ? profileSample
+          : undefined,
    });

    if (includeColumns && includeColumns?.length > 0) {
@ -287,11 +286,14 @@ const ProfilerSettingsModal: React.FC<ProfilerSettingsModalProps> = ({
      const profileConfig: TableProfilerConfig = {
        excludeColumns: excludeCol.length > 0 ? excludeCol : undefined,
        profileQuery: !isEmpty(sqlQuery) ? sqlQuery : undefined,
-        profileSample:
-          profileSampleType === ProfileSampleType.Percentage
+        profileSample: profileSampleType
+          ? profileSampleType === ProfileSampleType.Percentage
            ? profileSamplePercentage
-            : profileSampleRows,
-        profileSampleType: profileSampleType,
+            : profileSampleRows
+          : undefined,
+        profileSampleType: isUndefined(profileSampleType)
+          ? null
+          : profileSampleType,
        includeColumns: !isEqual(includeCol, DEFAULT_INCLUDE_PROFILE)
          ? getIncludesColumns()
          : undefined,
@ -461,31 +463,37 @@ const ProfilerSettingsModal: React.FC<ProfilerSettingsModalProps> = ({
                })}
                name="profileSampleType">
                <Select
+                  allowClear
                  autoFocus
                  className="w-full"
                  data-testid="profile-sample"
                  options={PROFILE_SAMPLE_OPTIONS}
+                  placeholder={t('label.please-select-entity', {
+                    entity: t('label.profile-sample-type', {
+                      type: '',
+                    }),
+                  })}
                  onChange={handleProfileSampleType}
                />
              </Form.Item>

              {state?.selectedProfileSampleType ===
-              ProfileSampleType.Percentage ? (
+                ProfileSampleType.Percentage && (
                <Form.Item
-                  className="m-b-0"
                  label={t('label.profile-sample-type', {
                    type: t('label.value'),
                  })}
                  name="profileSamplePercentage">
                  <SliderWithInput
                    className="p-x-xs"
-                    value={state?.profileSample || 0}
+                    value={state?.profileSample}
                    onChange={handleProfileSample}
                  />
                </Form.Item>
-              ) : (
+              )}
+
+              {state?.selectedProfileSampleType === ProfileSampleType.Rows && (
                <Form.Item
-                  className="m-b-0"
                  label={t('label.profile-sample-type', {
                    type: t('label.value'),
                  })}
--- a/openmetadata-ui/src/main/resources/ui/src/components/common/SliderWithInput/SliderWithInput.interface.ts
+++ b/openmetadata-ui/src/main/resources/ui/src/components/common/SliderWithInput/SliderWithInput.interface.ts
@ -12,7 +12,7 @@
 */

 export interface SliderWithInputProps {
-  value: number;
+  value?: number;
  onChange: (value: number | null) => void;
  className?: string;
 }
--- a/openmetadata-ui/src/main/resources/ui/src/components/common/SliderWithInput/SliderWithInput.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/components/common/SliderWithInput/SliderWithInput.tsx
@ -11,20 +11,22 @@
 *  limitations under the License.
 */

-import { Col, InputNumber, Row, Slider } from 'antd';
-import React, { useCallback } from 'react';
+import { CloseOutlined } from '@ant-design/icons';
+import { Button, Col, InputNumber, Row, Slider, Tooltip } from 'antd';
+import React from 'react';
+import { useTranslation } from 'react-i18next';
+import { percentageFormatter } from '../../../utils/ChartUtils';
 import { SliderWithInputProps } from './SliderWithInput.interface';
-
 const SliderWithInput = ({
  value,
  onChange,
  className,
 }: SliderWithInputProps) => {
-  const formatter = useCallback((value) => `${value}%`, [value]);
+  const { t } = useTranslation();

  return (
    <Row className={className} data-testid="percentage-input" gutter={20}>
-      <Col span={20}>
+      <Col flex="auto">
        <Slider
          marks={{
            0: '0%',
@ -37,16 +39,27 @@ const SliderWithInput = ({
          onChange={onChange}
        />
      </Col>
-      <Col span={4}>
-        <InputNumber
-          data-testid="slider-input"
-          formatter={formatter}
-          max={100}
-          min={0}
-          step={1}
-          value={value}
-          onChange={onChange}
-        />
+      <Col className="w-32">
+        <div className="flex items-center gap-2">
+          <InputNumber
+            data-testid="slider-input"
+            formatter={percentageFormatter}
+            max={100}
+            min={0}
+            step={1}
+            value={value}
+            onChange={onChange}
+          />
+          <Tooltip title={t('label.clear')}>
+            <Button
+              className="p-0"
+              data-testid="clear-slider-input"
+              type="text"
+              onClick={() => onChange(null)}>
+              <CloseOutlined />
+            </Button>
+          </Tooltip>
+        </div>
      </Col>
    </Row>
  );
--- a/openmetadata-ui/src/main/resources/ui/src/utils/ChartUtils.test.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/utils/ChartUtils.test.tsx
@ -0,0 +1,31 @@
+/*
+ *  Copyright 2025 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+import { percentageFormatter } from './ChartUtils';
+
+describe('ChartUtils', () => {
+  describe('percentageFormatter', () => {
+    it('should format number with percentage symbol', () => {
+      expect(percentageFormatter(50)).toBe('50%');
+      expect(percentageFormatter(100)).toBe('100%');
+    });
+
+    it('should handle decimal numbers', () => {
+      expect(percentageFormatter(50.5)).toBe('50.5%');
+      expect(percentageFormatter(33.33)).toBe('33.33%');
+    });
+
+    it('should return empty string for undefined value', () => {
+      expect(percentageFormatter(undefined)).toBe('');
+    });
+  });
+});
--- a/openmetadata-ui/src/main/resources/ui/src/utils/ChartUtils.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/utils/ChartUtils.tsx
@ -42,3 +42,7 @@ export const updateActiveChartFilter = (

  return updatedData;
 };
+
+export const percentageFormatter = (value?: number) => {
+  return value ? `${value}%` : '';
+};