You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/26 18:37:48 UTC

(superset) branch 4.0 updated (7c14968e6d -> 34b06f94ab)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a change to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git


    from 7c14968e6d fix(sql_parse): Ensure table extraction handles Jinja templating (#27470)
     new 3bfa8a9cbc fix(AlertReports): clearing custom_width when disabled (#27551)
     new d8517213bf fix(AlertReports): defaulting grace period to undefined (#27552)
     new 29553939a2 fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)
     new 32aa25ae17 fix(sqllab): unable to remove table (#27636)
     new 34b06f94ab fix(AlertReports): disabling value when not null option is active (#27550)

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 superset-frontend/src/SqlLab/actions/sqlLab.js     |   8 +-
 .../src/SqlLab/actions/sqlLab.test.js              |  24 +-
 .../AnnotationLayerControl/AnnotationLayer.jsx     | 350 ++++++++++++++-------
 .../AnnotationLayer.test.tsx                       | 125 +++++---
 .../src/features/alerts/AlertReportModal.test.tsx  |   5 +-
 .../src/features/alerts/AlertReportModal.tsx       |   8 +-
 6 files changed, 365 insertions(+), 155 deletions(-)


(superset) 01/05: fix(AlertReports): clearing custom_width when disabled (#27551)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 3bfa8a9cbca1aad58f95cefba05c85c74d8797fd
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Fri Mar 22 13:49:18 2024 -0500

    fix(AlertReports): clearing custom_width when disabled (#27551)
    
    (cherry picked from commit 0f6e4041c73bcae931bac0a9daa1837beac5aaf6)
---
 superset-frontend/src/features/alerts/AlertReportModal.tsx | 1 +
 1 file changed, 1 insertion(+)

diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index c297da2d71..c6acfa6aad 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -611,6 +611,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       chart: contentType === 'chart' ? currentAlert?.chart?.value : null,
       dashboard:
         contentType === 'dashboard' ? currentAlert?.dashboard?.value : null,
+      custom_width: isScreenshot ? currentAlert?.custom_width : undefined,
       database: currentAlert?.database?.value,
       owners: (currentAlert?.owners || []).map(
         owner => (owner as MetaObject).value || owner.id,


(superset) 03/05: fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 29553939a279fd9abaf36dcf0aabacaf1764d550
Author: Ross Mabbett <92...@users.noreply.github.com>
AuthorDate: Tue Mar 26 12:54:03 2024 -0400

    fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)
    
    (cherry picked from commit ce210eebdeeb374611e5b273379a889244f64288)
---
 .../AnnotationLayerControl/AnnotationLayer.jsx     | 350 ++++++++++++++-------
 .../AnnotationLayer.test.tsx                       | 125 +++++---
 2 files changed, 329 insertions(+), 146 deletions(-)

diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
index 563b946823..e29befc808 100644
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
@@ -33,12 +33,12 @@ import {
   withTheme,
 } from '@superset-ui/core';
 import SelectControl from 'src/explore/components/controls/SelectControl';
+import { AsyncSelect } from 'src/components';
 import TextControl from 'src/explore/components/controls/TextControl';
 import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
 import PopoverSection from 'src/components/PopoverSection';
 import ControlHeader from 'src/explore/components/ControlHeader';
 import { EmptyStateSmall } from 'src/components/EmptyState';
-import { FILTER_OPTIONS_LIMIT } from 'src/explore/constants';
 import {
   ANNOTATION_SOURCE_TYPES,
   ANNOTATION_TYPES,
@@ -194,28 +194,46 @@ class AnnotationLayer extends React.PureComponent {
       hideLine,
       // refData
       isNew: !name,
-      isLoadingOptions: true,
-      valueOptions: [],
+      slice: null,
     };
     this.submitAnnotation = this.submitAnnotation.bind(this);
     this.deleteAnnotation = this.deleteAnnotation.bind(this);
     this.applyAnnotation = this.applyAnnotation.bind(this);
-    this.fetchOptions = this.fetchOptions.bind(this);
+    this.isValidForm = this.isValidForm.bind(this);
+    // Handlers
     this.handleAnnotationType = this.handleAnnotationType.bind(this);
     this.handleAnnotationSourceType =
       this.handleAnnotationSourceType.bind(this);
-    this.handleValue = this.handleValue.bind(this);
-    this.isValidForm = this.isValidForm.bind(this);
+    this.handleSelectValue = this.handleSelectValue.bind(this);
+    this.handleTextValue = this.handleTextValue.bind(this);
+    // Fetch related functions
+    this.fetchOptions = this.fetchOptions.bind(this);
+    this.fetchCharts = this.fetchCharts.bind(this);
+    this.fetchNativeAnnotations = this.fetchNativeAnnotations.bind(this);
+    this.fetchAppliedAnnotation = this.fetchAppliedAnnotation.bind(this);
+    this.fetchSliceData = this.fetchSliceData.bind(this);
+    this.shouldFetchSliceData = this.shouldFetchSliceData.bind(this);
+    this.fetchAppliedChart = this.fetchAppliedChart.bind(this);
+    this.fetchAppliedNativeAnnotation =
+      this.fetchAppliedNativeAnnotation.bind(this);
+    this.shouldFetchAppliedAnnotation =
+      this.shouldFetchAppliedAnnotation.bind(this);
   }
 
   componentDidMount() {
-    const { annotationType, sourceType, isLoadingOptions } = this.state;
-    this.fetchOptions(annotationType, sourceType, isLoadingOptions);
+    if (this.shouldFetchAppliedAnnotation()) {
+      const { value } = this.state;
+      /* The value prop is the id of the chart/native. This function will set
+      value in state to an object with the id as value.value to be used by
+      AsyncSelect */
+      this.fetchAppliedAnnotation(value);
+    }
   }
 
   componentDidUpdate(prevProps, prevState) {
-    if (prevState.sourceType !== this.state.sourceType) {
-      this.fetchOptions(this.state.annotationType, this.state.sourceType, true);
+    if (this.shouldFetchSliceData(prevState)) {
+      const { value } = this.state;
+      this.fetchSliceData(value.value);
     }
   }
 
@@ -237,6 +255,20 @@ class AnnotationLayer extends React.PureComponent {
     return sources;
   }
 
+  shouldFetchAppliedAnnotation() {
+    const { value, sourceType } = this.state;
+    return value && requiresQuery(sourceType);
+  }
+
+  shouldFetchSliceData(prevState) {
+    const { value, sourceType } = this.state;
+    const isChart =
+      sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE &&
+      requiresQuery(sourceType);
+    const valueIsNew = value && prevState.value !== value;
+    return valueIsNew && isChart;
+  }
+
   isValidFormulaAnnotation(expression, annotationType) {
     if (annotationType === ANNOTATION_TYPES.FORMULA) {
       return isValidExpression(expression);
@@ -276,6 +308,7 @@ class AnnotationLayer extends React.PureComponent {
       annotationType,
       sourceType: null,
       value: null,
+      slice: null,
     });
   }
 
@@ -283,13 +316,17 @@ class AnnotationLayer extends React.PureComponent {
     const { sourceType: prevSourceType } = this.state;
 
     if (prevSourceType !== sourceType) {
-      this.setState({ sourceType, value: null, isLoadingOptions: true });
+      this.setState({
+        sourceType,
+        value: null,
+        slice: null,
+      });
     }
   }
 
-  handleValue(value) {
+  handleSelectValue(selectedValueObject) {
     this.setState({
-      value,
+      value: selectedValueObject,
       descriptionColumns: [],
       intervalEndColumn: null,
       timeColumn: null,
@@ -298,74 +335,172 @@ class AnnotationLayer extends React.PureComponent {
     });
   }
 
-  fetchOptions(annotationType, sourceType, isLoadingOptions) {
-    if (isLoadingOptions) {
-      if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
-        const queryParams = rison.encode({
-          page: 0,
-          page_size: FILTER_OPTIONS_LIMIT,
-        });
-        SupersetClient.get({
-          endpoint: `/api/v1/annotation_layer/?q=${queryParams}`,
-        }).then(({ json }) => {
-          const layers = json
-            ? json.result.map(layer => ({
-                value: layer.id,
-                label: layer.name,
-              }))
-            : [];
-          this.setState({
-            isLoadingOptions: false,
-            valueOptions: layers,
-          });
-        });
-      } else if (requiresQuery(sourceType)) {
-        const queryParams = rison.encode({
-          filters: [
-            {
-              col: 'id',
-              opr: 'chart_owned_created_favored_by_me',
-              value: true,
-            },
-          ],
-          order_column: 'slice_name',
-          order_direction: 'asc',
-          page: 0,
-          page_size: FILTER_OPTIONS_LIMIT,
-        });
-        SupersetClient.get({
-          endpoint: `/api/v1/chart/?q=${queryParams}`,
-        }).then(({ json }) => {
-          const registry = getChartMetadataRegistry();
-          this.setState({
-            isLoadingOptions: false,
-            valueOptions: json.result
-              .filter(x => {
-                const metadata = registry.get(x.viz_type);
-                return metadata && metadata.canBeAnnotationType(annotationType);
-              })
-              .map(x => ({
-                value: x.id,
-                label: x.slice_name,
-                slice: {
-                  ...x,
-                  data: {
-                    ...x.form_data,
-                    groupby: x.form_data.groupby?.map(column =>
-                      getColumnLabel(column),
-                    ),
-                  },
-                },
-              })),
-          });
-        });
-      } else {
+  handleTextValue(inputValue) {
+    this.setState({
+      value: inputValue,
+    });
+  }
+
+  fetchNativeAnnotations = async (search, page, pageSize) => {
+    const queryParams = rison.encode({
+      filters: [
+        {
+          col: 'name',
+          opr: 'ct',
+          value: search,
+        },
+      ],
+      columns: ['id', 'name'],
+      page,
+      page_size: pageSize,
+    });
+
+    const { json } = await SupersetClient.get({
+      endpoint: `/api/v1/annotation_layer/?q=${queryParams}`,
+    });
+
+    const { result, count } = json;
+
+    const layersArray = result.map(layer => ({
+      value: layer.id,
+      label: layer.name,
+    }));
+
+    return {
+      data: layersArray,
+      totalCount: count,
+    };
+  };
+
+  fetchCharts = async (search, page, pageSize) => {
+    const { annotationType } = this.state;
+
+    const queryParams = rison.encode({
+      filters: [
+        { col: 'slice_name', opr: 'chart_all_text', value: search },
+        {
+          col: 'id',
+          opr: 'chart_owned_created_favored_by_me',
+          value: true,
+        },
+      ],
+      columns: ['id', 'slice_name', 'viz_type'],
+      order_column: 'slice_name',
+      order_direction: 'asc',
+      page,
+      page_size: pageSize,
+    });
+    const { json } = await SupersetClient.get({
+      endpoint: `/api/v1/chart/?q=${queryParams}`,
+    });
+
+    const { result, count } = json;
+    const registry = getChartMetadataRegistry();
+
+    const chartsArray = result
+      .filter(chart => {
+        const metadata = registry.get(chart.viz_type);
+        return metadata && metadata.canBeAnnotationType(annotationType);
+      })
+      .map(chart => ({
+        value: chart.id,
+        label: chart.slice_name,
+        viz_type: chart.viz_type,
+      }));
+
+    return {
+      data: chartsArray,
+      totalCount: count,
+    };
+  };
+
+  fetchOptions = (search, page, pageSize) => {
+    const { sourceType } = this.state;
+
+    if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
+      return this.fetchNativeAnnotations(search, page, pageSize);
+    }
+    return this.fetchCharts(search, page, pageSize);
+  };
+
+  fetchSliceData = id => {
+    const queryParams = rison.encode({
+      columns: ['query_context'],
+    });
+    SupersetClient.get({
+      endpoint: `/api/v1/chart/${id}?q=${queryParams}`,
+    }).then(({ json }) => {
+      const { result } = json;
+      const queryContext = result.query_context;
+      const formData = JSON.parse(queryContext).form_data;
+      const dataObject = {
+        data: {
+          ...formData,
+          groupby: formData.groupby?.map(column => getColumnLabel(column)),
+        },
+      };
+      this.setState({
+        slice: dataObject,
+      });
+    });
+  };
+
+  fetchAppliedChart(id) {
+    const { annotationType } = this.state;
+    const registry = getChartMetadataRegistry();
+    const queryParams = rison.encode({
+      columns: ['slice_name', 'query_context', 'viz_type'],
+    });
+    SupersetClient.get({
+      endpoint: `/api/v1/chart/${id}?q=${queryParams}`,
+    }).then(({ json }) => {
+      const { result } = json;
+      const sliceName = result.slice_name;
+      const queryContext = result.query_context;
+      const vizType = result.viz_type;
+      const formData = JSON.parse(queryContext).form_data;
+      const metadata = registry.get(vizType);
+      const canBeAnnotationType =
+        metadata && metadata.canBeAnnotationType(annotationType);
+      if (canBeAnnotationType) {
         this.setState({
-          isLoadingOptions: false,
-          valueOptions: [],
+          value: {
+            value: id,
+            label: sliceName,
+          },
+          slice: {
+            data: {
+              ...formData,
+              groupby: formData.groupby?.map(column => getColumnLabel(column)),
+            },
+          },
         });
       }
+    });
+  }
+
+  fetchAppliedNativeAnnotation(id) {
+    SupersetClient.get({
+      endpoint: `/api/v1/annotation_layer/${id}`,
+    }).then(({ json }) => {
+      const { result } = json;
+      const layer = result;
+      this.setState({
+        value: {
+          value: layer.id,
+          label: layer.name,
+        },
+      });
+    });
+  }
+
+  fetchAppliedAnnotation(id) {
+    const { sourceType } = this.state;
+
+    if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
+      return this.fetchAppliedNativeAnnotation(id);
     }
+    return this.fetchAppliedChart(id);
   }
 
   deleteAnnotation() {
@@ -374,6 +509,7 @@ class AnnotationLayer extends React.PureComponent {
   }
 
   applyAnnotation() {
+    const { value, sourceType } = this.state;
     if (this.isValidForm()) {
       const annotationFields = [
         'name',
@@ -385,7 +521,6 @@ class AnnotationLayer extends React.PureComponent {
         'width',
         'showMarkers',
         'hideLine',
-        'value',
         'overrides',
         'show',
         'showLabel',
@@ -401,6 +536,10 @@ class AnnotationLayer extends React.PureComponent {
         }
       });
 
+      // Prepare newAnnotation.value for use in runAnnotationQuery()
+      const applicableValue = requiresQuery(sourceType) ? value.value : value;
+      newAnnotation.value = applicableValue;
+
       if (newAnnotation.color === AUTOMATIC_COLOR) {
         newAnnotation.color = null;
       }
@@ -415,29 +554,19 @@ class AnnotationLayer extends React.PureComponent {
     this.props.close();
   }
 
-  renderOption(option) {
+  renderChartHeader(label, description, value) {
     return (
-      <span
-        css={{
-          overflow: 'hidden',
-          textOverflow: 'ellipsis',
-          whiteSpace: 'nowrap',
-        }}
-        title={option.label}
-      >
-        {option.label}
-      </span>
+      <ControlHeader
+        hovered
+        label={label}
+        description={description}
+        validationErrors={!value ? ['Mandatory'] : []}
+      />
     );
   }
 
   renderValueConfiguration() {
-    const {
-      annotationType,
-      sourceType,
-      value,
-      valueOptions,
-      isLoadingOptions,
-    } = this.state;
+    const { annotationType, sourceType, value } = this.state;
     let label = '';
     let description = '';
     if (requiresQuery(sourceType)) {
@@ -462,20 +591,15 @@ class AnnotationLayer extends React.PureComponent {
     }
     if (requiresQuery(sourceType)) {
       return (
-        <SelectControl
+        <AsyncSelect
+          /* key to force re-render on sourceType change */
+          key={sourceType}
           ariaLabel={t('Annotation layer value')}
           name="annotation-layer-value"
-          showHeader
-          hovered
-          description={description}
-          label={label}
-          placeholder=""
-          options={valueOptions}
-          isLoading={isLoadingOptions}
-          value={value}
-          onChange={this.handleValue}
-          validationErrors={!value ? ['Mandatory'] : []}
-          optionRenderer={this.renderOption}
+          header={this.renderChartHeader(label, description, value)}
+          options={this.fetchOptions}
+          value={value || null}
+          onChange={this.handleSelectValue}
           notFoundContent={<NotFoundContent />}
         />
       );
@@ -490,7 +614,7 @@ class AnnotationLayer extends React.PureComponent {
           label={label}
           placeholder=""
           value={value}
-          onChange={this.handleValue}
+          onChange={this.handleTextValue}
           validationErrors={
             !this.isValidFormulaAnnotation(value, annotationType)
               ? [t('Bad formula.')]
@@ -507,14 +631,18 @@ class AnnotationLayer extends React.PureComponent {
       annotationType,
       sourceType,
       value,
-      valueOptions,
+      slice,
       overrides,
       titleColumn,
       timeColumn,
       intervalEndColumn,
       descriptionColumns,
     } = this.state;
-    const { slice } = valueOptions.find(x => x.value === value) || {};
+
+    if (!slice || !value) {
+      return '';
+    }
+
     if (sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE && slice) {
       const columns = (slice.data.groupby || [])
         .concat(slice.data.all_columns || [])
diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
index 914be73619..813a8ebcb9 100644
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
@@ -31,21 +31,37 @@ const defaultProps = {
   annotationType: ANNOTATION_TYPES_METADATA.FORMULA.value,
 };
 
+const nativeLayerApiRoute = 'glob:*/api/v1/annotation_layer/*';
+const chartApiRoute = /\/api\/v1\/chart\/\?q=.+/;
+const chartApiWithIdRoute = /\/api\/v1\/chart\/\w+\?q=.+/;
+
+const withIdResult = {
+  result: {
+    slice_name: 'Mocked Slice',
+    query_context: JSON.stringify({
+      form_data: {
+        groupby: ['country'],
+      },
+    }),
+    viz_type: 'line',
+  },
+};
+
 beforeAll(() => {
   const supportedAnnotationTypes = Object.values(ANNOTATION_TYPES_METADATA).map(
     value => value.value,
   );
 
-  fetchMock.get('glob:*/api/v1/annotation_layer/*', {
-    result: [{ label: 'Chart A', value: 'a' }],
+  fetchMock.get(nativeLayerApiRoute, {
+    result: [{ name: 'Chart A', id: 'a' }],
   });
 
-  fetchMock.get('glob:*/api/v1/chart/*', {
-    result: [
-      { id: 'a', slice_name: 'Chart A', viz_type: 'table', form_data: {} },
-    ],
+  fetchMock.get(chartApiRoute, {
+    result: [{ id: 'a', slice_name: 'Chart A', viz_type: 'table' }],
   });
 
+  fetchMock.get(chartApiWithIdRoute, withIdResult);
+
   setupColors();
 
   getChartMetadataRegistry().registerValue(
@@ -144,7 +160,7 @@ test('triggers removeAnnotationLayer and close when remove button is clicked', a
   expect(close).toHaveBeenCalled();
 });
 
-test('renders chart options', async () => {
+test('fetches Superset annotation layer options', async () => {
   await waitForRender({
     annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
   });
@@ -153,12 +169,37 @@ test('renders chart options', async () => {
   );
   userEvent.click(screen.getByText('Superset annotation'));
   expect(await screen.findByText('Annotation layer')).toBeInTheDocument();
+  userEvent.click(
+    screen.getByRole('combobox', { name: 'Annotation layer value' }),
+  );
+  expect(await screen.findByText('Chart A')).toBeInTheDocument();
+  expect(fetchMock.calls(nativeLayerApiRoute).length).toBe(1);
+});
 
+test('fetches chart options', async () => {
+  await waitForRender({
+    annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
+  });
   userEvent.click(
     screen.getByRole('combobox', { name: 'Annotation source type' }),
   );
   userEvent.click(screen.getByText('Table'));
   expect(await screen.findByText('Chart')).toBeInTheDocument();
+  userEvent.click(
+    screen.getByRole('combobox', { name: 'Annotation layer value' }),
+  );
+  expect(await screen.findByText('Chart A')).toBeInTheDocument();
+  expect(fetchMock.calls(chartApiRoute).length).toBe(1);
+});
+
+test('fetches chart on mount if value present', async () => {
+  await waitForRender({
+    name: 'Test',
+    value: 'a',
+    annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
+    sourceType: 'Table',
+  });
+  expect(fetchMock.calls(chartApiWithIdRoute).length).toBe(1);
 });
 
 test('keeps apply disabled when missing required fields', async () => {
@@ -171,7 +212,7 @@ test('keeps apply disabled when missing required fields', async () => {
   );
   expect(await screen.findByText('Chart A')).toBeInTheDocument();
   userEvent.click(screen.getByText('Chart A'));
-
+  await screen.findByText(/title column/i);
   userEvent.click(screen.getByRole('button', { name: 'Automatic Color' }));
   userEvent.click(
     screen.getByRole('combobox', { name: 'Annotation layer title column' }),
@@ -197,47 +238,61 @@ test('keeps apply disabled when missing required fields', async () => {
   expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
 });
 
-test.skip('Disable apply button if formula is incorrect', async () => {
-  // TODO: fix flaky test that passes locally but fails on CI
+test('Disable apply button if formula is incorrect', async () => {
   await waitForRender({ name: 'test' });
 
-  userEvent.clear(screen.getByLabelText('Formula'));
-  userEvent.type(screen.getByLabelText('Formula'), 'x+1');
+  const formulaInput = screen.getByRole('textbox', { name: 'Formula' });
+  const applyButton = screen.getByRole('button', { name: 'Apply' });
+  const okButton = screen.getByRole('button', { name: 'OK' });
+
+  userEvent.type(formulaInput, 'x+1');
+  expect(formulaInput).toHaveValue('x+1');
   await waitFor(() => {
-    expect(screen.getByLabelText('Formula')).toHaveValue('x+1');
-    expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
-    expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
+    expect(okButton).toBeEnabled();
+    expect(applyButton).toBeEnabled();
   });
 
-  userEvent.clear(screen.getByLabelText('Formula'));
-  userEvent.type(screen.getByLabelText('Formula'), 'y = x*2+1');
+  userEvent.clear(formulaInput);
   await waitFor(() => {
-    expect(screen.getByLabelText('Formula')).toHaveValue('y = x*2+1');
-    expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
-    expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
+    expect(formulaInput).toHaveValue('');
+  });
+  userEvent.type(formulaInput, 'y = x*2+1');
+  expect(formulaInput).toHaveValue('y = x*2+1');
+  await waitFor(() => {
+    expect(okButton).toBeEnabled();
+    expect(applyButton).toBeEnabled();
   });
 
-  userEvent.clear(screen.getByLabelText('Formula'));
-  userEvent.type(screen.getByLabelText('Formula'), 'y+1');
+  userEvent.clear(formulaInput);
+  await waitFor(() => {
+    expect(formulaInput).toHaveValue('');
+  });
+  userEvent.type(formulaInput, 'y+1');
+  expect(formulaInput).toHaveValue('y+1');
   await waitFor(() => {
-    expect(screen.getByLabelText('Formula')).toHaveValue('y+1');
-    expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
-    expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+    expect(okButton).toBeDisabled();
+    expect(applyButton).toBeDisabled();
   });
 
-  userEvent.clear(screen.getByLabelText('Formula'));
-  userEvent.type(screen.getByLabelText('Formula'), 'x+');
+  userEvent.clear(formulaInput);
   await waitFor(() => {
-    expect(screen.getByLabelText('Formula')).toHaveValue('x+');
-    expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
-    expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+    expect(formulaInput).toHaveValue('');
+  });
+  userEvent.type(formulaInput, 'x+');
+  expect(formulaInput).toHaveValue('x+');
+  await waitFor(() => {
+    expect(okButton).toBeDisabled();
+    expect(applyButton).toBeDisabled();
   });
 
-  userEvent.clear(screen.getByLabelText('Formula'));
-  userEvent.type(screen.getByLabelText('Formula'), 'y = z+1');
+  userEvent.clear(formulaInput);
+  await waitFor(() => {
+    expect(formulaInput).toHaveValue('');
+  });
+  userEvent.type(formulaInput, 'y = z+1');
+  expect(formulaInput).toHaveValue('y = z+1');
   await waitFor(() => {
-    expect(screen.getByLabelText('Formula')).toHaveValue('y = z+1');
-    expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
-    expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+    expect(okButton).toBeDisabled();
+    expect(applyButton).toBeDisabled();
   });
 });


(superset) 05/05: fix(AlertReports): disabling value when not null option is active (#27550)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 34b06f94ab07f5c334c30d1da239700cb2e6cbbe
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Tue Mar 26 13:13:29 2024 -0500

    fix(AlertReports): disabling value when not null option is active (#27550)
    
    (cherry picked from commit ed9e5427817312b1b706e4e8ada3ecd78b9b79d5)
---
 superset-frontend/src/features/alerts/AlertReportModal.test.tsx | 5 ++++-
 superset-frontend/src/features/alerts/AlertReportModal.tsx      | 5 +++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
index 358aa27df3..70d01885c3 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
@@ -371,12 +371,15 @@ test('disables condition threshold if not null condition is selected', async ()
   userEvent.click(screen.getByTestId('alert-condition-panel'));
   await screen.findByText(/smaller than/i);
   const condition = screen.getByRole('combobox', { name: /condition/i });
+  const spinButton = screen.getByRole('spinbutton');
+  expect(spinButton).toHaveValue(10);
   await comboboxSelect(
     condition,
     'not null',
     () => screen.getAllByText(/not null/i)[0],
   );
-  expect(screen.getByRole('spinbutton')).toBeDisabled();
+  expect(spinButton).toHaveValue(null);
+  expect(spinButton).toBeDisabled();
 });
 
 // Content Section
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index aa0f123228..58b7b72d60 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -1417,7 +1417,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
               </StyledInputContainer>
               <StyledInputContainer css={noMarginBottom}>
                 <div className="control-label">
-                  {t('Value')} <span className="required">*</span>
+                  {t('Value')}{' '}
+                  {!conditionNotNull && <span className="required">*</span>}
                 </div>
                 <div className="input-container">
                   <input
@@ -1426,7 +1427,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                     disabled={conditionNotNull}
                     value={
                       currentAlert?.validator_config_json?.threshold !==
-                      undefined
+                        undefined && !conditionNotNull
                         ? currentAlert.validator_config_json.threshold
                         : ''
                     }


(superset) 02/05: fix(AlertReports): defaulting grace period to undefined (#27552)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit d8517213bfb781eea7a75264823eea24cc779090
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Mon Mar 25 12:12:01 2024 -0500

    fix(AlertReports): defaulting grace period to undefined (#27552)
    
    (cherry picked from commit 4fce940a9c3566c5dded68aa5cbba26fb562ae69)
---
 superset-frontend/src/features/alerts/AlertReportModal.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index c6acfa6aad..aa0f123228 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -886,7 +886,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
 
     // Need to make sure grace period is not lower than TIMEOUT_MIN
     if (value === 0) {
-      updateAlertState(target.name, null);
+      updateAlertState(target.name, undefined);
     } else {
       updateAlertState(
         target.name,


(superset) 04/05: fix(sqllab): unable to remove table (#27636)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 4.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 32aa25ae176cc42fccf3b45fef821a8f95cc33da
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Tue Mar 26 10:19:50 2024 -0700

    fix(sqllab): unable to remove table (#27636)
    
    (cherry picked from commit fa3fea9dd811d3cfdbbfe93f31d34992e603ec60)
---
 superset-frontend/src/SqlLab/actions/sqlLab.js     |  8 +++++---
 .../src/SqlLab/actions/sqlLab.test.js              | 24 ++++++++++++++++++++--
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
index 16bf3f530f..540a0e8536 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -1130,9 +1130,11 @@ export function removeTables(tables) {
     const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
       ? Promise.all(
           tablesToRemove.map(table =>
-            SupersetClient.delete({
-              endpoint: encodeURI(`/tableschemaview/${table.id}`),
-            }),
+            table.initialized
+              ? SupersetClient.delete({
+                  endpoint: encodeURI(`/tableschemaview/${table.id}`),
+                })
+              : Promise.resolve(),
           ),
         )
       : Promise.resolve();
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
index dd48ed8c7b..20fd53ad38 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -883,7 +883,7 @@ describe('async actions', () => {
       it('updates the table schema state in the backend', () => {
         expect.assertions(2);
 
-        const table = { id: 1 };
+        const table = { id: 1, initialized: true };
         const store = mockStore({});
         const expectedActions = [
           {
@@ -900,7 +900,10 @@ describe('async actions', () => {
       it('deletes multiple tables and updates the table schema state in the backend', () => {
         expect.assertions(2);
 
-        const tables = [{ id: 1 }, { id: 2 }];
+        const tables = [
+          { id: 1, initialized: true },
+          { id: 2, initialized: true },
+        ];
         const store = mockStore({});
         const expectedActions = [
           {
@@ -913,6 +916,23 @@ describe('async actions', () => {
           expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(2);
         });
       });
+
+      it('only updates the initialized table schema state in the backend', () => {
+        expect.assertions(2);
+
+        const tables = [{ id: 1 }, { id: 2, initialized: true }];
+        const store = mockStore({});
+        const expectedActions = [
+          {
+            type: actions.REMOVE_TABLES,
+            tables,
+          },
+        ];
+        return store.dispatch(actions.removeTables(tables)).then(() => {
+          expect(store.getActions()).toEqual(expectedActions);
+          expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1);
+        });
+      });
     });
 
     describe('migrateQueryEditorFromLocalStorage', () => {