test(explore): fix and improve PropertiesModal tests

- Fix TypeError: use jest.spyOn(SupersetClient, 'put') instead of
  fetchMock.calls (which does not exist on this version of fetch-mock)
- Remove cache_timeout from GET mock result so onSave assertion is correct
- Consolidate PUT body + onSave payload assertions into one test
- Add subtitle text tests: 'Name, description, and ownership' and
  'Mark this chart as certified'
- Add placeholder text tests for Name, Description, Certified by,
  Certification details inputs
- Split helper text test: separate Description/Owners and Tags tests
- Add Tags helper text test gated behind TaggingSystem feature flag

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx

@@ -24,7 +24,7 @@ import {
   waitFor,
 } from 'spec/helpers/testing-library';
 import fetchMock from 'fetch-mock';
-import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core';
+import { isFeatureEnabled, FeatureFlag, SupersetClient } from '@superset-ui/core';
 import PropertiesModal, { PropertiesModalProps } from '.';
 
 jest.mock('@superset-ui/core', () => ({
@@ -91,7 +91,6 @@ fetchMock.get('glob:*/api/v1/chart/318*', {
       certification_details: 'Test certification details',
       certified_by: 'Test certified by',
       description: 'Test description',
-      cache_timeout: 1000,
       slice_name: 'Test chart new name',
     },
     show_columns: [
@@ -204,12 +203,14 @@ test('Should render all elements inside modal', async () => {
       // Check we have the modal
       expect(screen.getByRole('dialog')).toBeInTheDocument();
 
-      // Check for collapse sections instead of expecting all textboxes to be visible
-      expect(screen.getByText('General settings')).toBeInTheDocument();
-      expect(screen.getByText('Configuration')).toBeInTheDocument();
-      expect(screen.getByText('Advanced')).toBeInTheDocument();
+      // Check for collapse sections
+      expect(screen.getByText('Basic info')).toBeInTheDocument();
+      expect(screen.getByText('Certification')).toBeInTheDocument();
 
-      // Only General settings is expanded by default
+      // Configuration section should not be present
+      expect(screen.queryByText('Configuration')).not.toBeInTheDocument();
+
+      // Only Basic info is expanded by default
       // Check for visible labels and fields in the expanded section
       expect(screen.getByText('Name')).toBeInTheDocument();
       expect(screen.getByText('Description')).toBeInTheDocument();
@@ -284,8 +285,10 @@ test('Empty "Certified by" should clear "Certification details"', async () => {
   };
   renderModal(noCertifiedByProps);
 
-  // Expand the Advanced section first to access certification details
-  const advancedPanel = screen.getByText('Advanced').closest('[role="tab"]');
+  // Expand the Certification section first to access certification details
+  const advancedPanel = screen
+    .getByText('Certification')
+    .closest('[role="tab"]');
   if (advancedPanel) {
     await userEvent.click(advancedPanel);
   }
@@ -338,31 +341,43 @@ test('"Name" should not be empty when saved', async () => {
   });
 });
 
-test('"Cache timeout" should not be empty when saved', async () => {
+test('"Cache timeout" field and Configuration section should not be present', () => {
   const props = createProps();
   renderModal(props);
 
-  // Expand the Configuration section first to access cache timeout
-  const configPanel = screen.getByText('Configuration').closest('[role="tab"]');
-  if (configPanel) {
-    await userEvent.click(configPanel);
-  }
+  expect(
+    screen.queryByRole('textbox', { name: 'Cache timeout' }),
+  ).not.toBeInTheDocument();
+  expect(screen.queryByText('Configuration')).not.toBeInTheDocument();
+});
 
-  const cacheTimeout = await screen.findByRole('textbox', {
-    name: 'Cache timeout',
+test('"Save" should not include cache_timeout in the PUT body or onSave payload', async () => {
+  const props = createProps();
+  const putSpy = jest.spyOn(SupersetClient, 'put');
+  renderModal(props);
+
+  await waitFor(() => {
+    expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled();
   });
-  await userEvent.clear(cacheTimeout);
-  await userEvent.type(cacheTimeout, '1000');
-  expect(cacheTimeout).toHaveValue('1000');
 
   await userEvent.click(screen.getByRole('button', { name: 'Save' }));
 
   await waitFor(() => {
     expect(props.onSave).toHaveBeenCalledTimes(1);
-    expect(props.onSave).toHaveBeenCalledWith(
-      expect.objectContaining({ cache_timeout: 1000 }),
-    );
   });
+
+  // Verify cache_timeout was not sent in the PUT request body
+  expect(putSpy).toHaveBeenCalledTimes(1);
+  const putOptions = putSpy.mock.calls[0][0] as { body: string };
+  const putBody = JSON.parse(putOptions.body);
+  expect(putBody).not.toHaveProperty('cache_timeout');
+
+  // Verify cache_timeout is not in the object passed to the onSave callback
+  expect(props.onSave).not.toHaveBeenCalledWith(
+    expect.objectContaining({ cache_timeout: expect.anything() }),
+  );
+
+  putSpy.mockRestore();
 });
 
 test('"Description" should not be empty when saved', async () => {
@@ -397,8 +412,10 @@ test('"Certified by" should not be empty when saved', async () => {
   const props = createProps();
   renderModal(props);
 
-  // Expand the Advanced section first to access certified by
-  const advancedPanel = screen.getByText('Advanced').closest('[role="tab"]');
+  // Expand the Certification section first to access certified by
+  const advancedPanel = screen
+    .getByText('Certification')
+    .closest('[role="tab"]');
   if (advancedPanel) {
     await userEvent.click(advancedPanel);
   }
@@ -424,8 +441,10 @@ test('"Certification details" should not be empty when saved', async () => {
   const props = createProps();
   renderModal(props);
 
-  // Expand the Advanced section first to access certification details
-  const advancedPanel = screen.getByText('Advanced').closest('[role="tab"]');
+  // Expand the Certification section first to access certification details
+  const advancedPanel = screen
+    .getByText('Certification')
+    .closest('[role="tab"]');
   if (advancedPanel) {
     await userEvent.click(advancedPanel);
   }
@@ -449,6 +468,89 @@ test('"Certification details" should not be empty when saved', async () => {
   });
 });
 
+test('Should render correct section subtitles', async () => {
+  const props = createProps();
+  renderModal(props);
+
+  await waitFor(() => {
+    expect(
+      screen.getByText('Name, description, and ownership'),
+    ).toBeInTheDocument();
+    expect(screen.getByText('Mark this chart as certified')).toBeInTheDocument();
+  });
+});
+
+test('Should render correct placeholder text on input fields', async () => {
+  const props = createProps();
+  renderModal(props);
+
+  await waitFor(() => {
+    expect(
+      screen.getByRole('textbox', { name: 'Name' }),
+    ).toHaveAttribute('placeholder', 'Enter a name for this chart');
+    expect(
+      screen.getByRole('textbox', { name: 'Description' }),
+    ).toHaveAttribute('placeholder', 'Add a description for this chart');
+  });
+
+  // Expand Certification section to check its placeholders
+  const certPanel = screen
+    .getByText('Certification')
+    .closest('[role="tab"]');
+  if (certPanel) {
+    await userEvent.click(certPanel);
+  }
+
+  await waitFor(() => {
+    expect(
+      screen.getByRole('textbox', { name: 'Certified by' }),
+    ).toHaveAttribute('placeholder', 'Name of certifying person or team');
+    expect(
+      screen.getByRole('textbox', { name: 'Certification details' }),
+    ).toHaveAttribute('placeholder', 'Describe the certification');
+  });
+});
+
+test('Should render updated helper text for Description and Owners fields', async () => {
+  const props = createProps();
+  renderModal(props);
+
+  await waitFor(() => {
+    expect(
+      screen.getByText(
+        'Add context to this chart. Supports markdown. The description can be displayed as a header in dashboard view.',
+      ),
+    ).toBeInTheDocument();
+    expect(
+      screen.getByText(
+        'Users who can modify this chart. Searchable by name or username.',
+      ),
+    ).toBeInTheDocument();
+  });
+});
+
+test('Should render correct Tags helper text when tagging system is enabled', async () => {
+  const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
+    typeof isFeatureEnabled
+  >;
+  mockIsFeatureEnabled.mockImplementation(
+    flag => flag === FeatureFlag.TaggingSystem,
+  );
+
+  const props = createProps();
+  renderModal(props);
+
+  await waitFor(() => {
+    expect(
+      screen.getByText(
+        'Tags applied to this chart for organization and discovery.',
+      ),
+    ).toBeInTheDocument();
+  });
+
+  mockIsFeatureEnabled.mockRestore();
+});
+
 test('Should display only custom tags when tagging system is enabled', async () => {
   const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
     typeof isFeatureEnabled

superset-frontend/src/explore/components/PropertiesModal/index.tsx

@@ -74,9 +74,6 @@ function PropertiesModal({
   // values of form inputs
   const [name, setName] = useState(slice.slice_name || '');
   const [description, setDescription] = useState(slice.description || '');
-  const [cacheTimeout, setCacheTimeout] = useState(
-    slice.cache_timeout != null ? String(slice.cache_timeout) : '',
-  );
   const [certifiedBy, setCertifiedBy] = useState(slice.certified_by || '');
   const [certificationDetails, setCertificationDetails] = useState(
     slice.certified_by && slice.certification_details
@@ -93,7 +90,7 @@ function PropertiesModal({
     () => [
       {
         key: 'general',
-        name: t('General settings'),
+        name: t('Basic info'),
         validator: () => {
           const errors = [];
           if (!name || name.trim().length === 0) {
@@ -102,24 +99,13 @@ function PropertiesModal({
           return errors;
         },
       },
-      {
-        key: 'configuration',
-        name: t('Configuration'),
-        validator: () => {
-          const errors = [];
-          if (cacheTimeout && Number.isNaN(Number(cacheTimeout))) {
-            errors.push(t('Cache timeout must be a number'));
-          }
-          return errors;
-        },
-      },
       {
         key: 'advanced',
-        name: t('Advanced'),
+        name: t('Certification'),
         validator: () => [],
       },
     ],
-    [name, cacheTimeout],
+    [name],
   );
 
   const {
@@ -249,7 +235,6 @@ function PropertiesModal({
     const payload: { [key: string]: any } = {
       slice_name: name || null,
       description: description || null,
-      cache_timeout: cacheTimeout ? Number(cacheTimeout) : null,
       certified_by: certifiedBy || null,
       certification_details:
         certifiedBy && certificationDetails ? certificationDetails : null,
@@ -303,11 +288,6 @@ function PropertiesModal({
     validateSection('general');
   }, [name, validateSection]);
 
-  // Validate configuration section when cache timeout changes
-  useEffect(() => {
-    validateSection('configuration');
-  }, [cacheTimeout, validateSection]);
-
   const handleChangeTags = (tags: { label: string; value: number }[]) => {
     const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({
       id: r.value,
@@ -349,8 +329,8 @@ function PropertiesModal({
             key: 'general',
             label: (
               <CollapseLabelInModal
-                title={t('General settings')}
-                subtitle={t('Basic information about the chart')}
+                title={t('Basic info')}
+                subtitle={t('Name, description, and ownership')}
                 validateCheckStatus={!validationStatus.general?.hasErrors}
                 testId="general-section"
               />
@@ -371,6 +351,7 @@ function PropertiesModal({
                     data-test="properties-modal-name-input"
                     type="text"
                     value={name}
+                    placeholder={t('Enter a name for this chart')}
                     onChange={(event: ChangeEvent<HTMLInputElement>) =>
                       setName(event.target.value ?? '')
                     }
@@ -379,12 +360,13 @@ function PropertiesModal({
                 <ModalFormField
                   label={t('Description')}
                   helperText={t(
-                    'The description can be displayed as widget headers in the dashboard view. Supports markdown.',
+                    'Add context to this chart. Supports markdown. The description can be displayed as a header in dashboard view.',
                   )}
                 >
                   <Input.TextArea
                     rows={3}
                     value={description}
+                    placeholder={t('Add a description for this chart')}
                     onChange={(event: ChangeEvent<HTMLTextAreaElement>) =>
                       setDescription(event.target.value ?? '')
                     }
@@ -393,7 +375,7 @@ function PropertiesModal({
                 <ModalFormField
                   label={t('Owners')}
                   helperText={t(
-                    'A list of users who can alter the chart. Searchable by name or username.',
+                    'Users who can modify this chart. Searchable by name or username.',
                   )}
                 >
                   <AsyncSelect
@@ -412,7 +394,7 @@ function PropertiesModal({
                   <ModalFormField
                     label={t('Tags')}
                     helperText={t(
-                      'A list of tags that have been applied to this chart.',
+                      'Tags applied to this chart for organization and discovery.',
                     )}
                     bottomSpacing={false}
                   >
@@ -430,47 +412,12 @@ function PropertiesModal({
               </>
             ),
           },
-          {
-            key: 'configuration',
-            label: (
-              <CollapseLabelInModal
-                title={t('Configuration')}
-                subtitle={t('Configure caching and performance settings')}
-                validateCheckStatus={!validationStatus.configuration?.hasErrors}
-                testId="configuration-section"
-              />
-            ),
-            children: (
-              <ModalFormField
-                label={t('Cache timeout')}
-                helperText={t(
-                  "Duration (in seconds) of the caching timeout for this chart. Set to -1 to bypass the cache. Note this defaults to the dataset's timeout if undefined.",
-                )}
-                error={
-                  validationStatus.configuration?.hasErrors &&
-                  cacheTimeout &&
-                  Number.isNaN(Number(cacheTimeout))
-                    ? t('Cache timeout must be a number')
-                    : undefined
-                }
-                bottomSpacing={false}
-              >
-                <Input
-                  aria-label={t('Cache timeout')}
-                  value={cacheTimeout}
-                  onChange={(event: ChangeEvent<HTMLInputElement>) =>
-                    setCacheTimeout(event.target.value ?? '')
-                  }
-                />
-              </ModalFormField>
-            ),
-          },
           {
             key: 'advanced',
             label: (
               <CollapseLabelInModal
-                title={t('Advanced')}
-                subtitle={t('Certification and additional settings')}
+                title={t('Certification')}
+                subtitle={t('Mark this chart as certified')}
                 validateCheckStatus={!validationStatus.advanced?.hasErrors}
                 testId="advanced-section"
               />
@@ -480,12 +427,13 @@ function PropertiesModal({
                 <ModalFormField
                   label={t('Certified by')}
                   helperText={t(
-                    'Person or group that has certified this chart.',
+                    'Person or team that has certified this chart.',
                   )}
                 >
                   <Input
                     aria-label={t('Certified by')}
                     value={certifiedBy}
+                    placeholder={t('Name of certifying person or team')}
                     onChange={(event: ChangeEvent<HTMLInputElement>) =>
                       setCertifiedBy(event.target.value ?? '')
                     }
@@ -494,13 +442,14 @@ function PropertiesModal({
                 <ModalFormField
                   label={t('Certification details')}
                   helperText={t(
-                    'Any additional detail to show in the certification tooltip.',
+                    'Additional details shown in the certification tooltip.',
                   )}
                   bottomSpacing={false}
                 >
                   <Input
                     aria-label={t('Certification details')}
                     value={certificationDetails}
+                    placeholder={t('Describe the certification')}
                     onChange={(event: ChangeEvent<HTMLInputElement>) =>
                       setCertificationDetails(event.target.value ?? '')
                     }