You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by am...@apache.org on 2021/05/07 18:38:38 UTC

[superset] branch 1.2 updated (963e79f -> f2236fc)

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

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


    from 963e79f  chore: bump table plugin to 0.17.42 (#14460)
     new f8491df  fix(annotations): pass force param to annotation request (#14483)
     new 099514d  fix: SQL Statement on QUERY_LOGGER prints none to log (#14358)
     new c2d2478  fix: parameterize titles correctly (#14509)
     new d0b683c  feat(native-filters): Auto apply changes in FiltersConfigModal (#14461)
     new d0a6706  feat(dremio): implement convert_dttm method (#14519)
     new f2236fc  fix(chart-data): handle url_params in csv export and native filters (#14526)

The 6 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/chart/chartAction.js         | 17 ++++-
 superset-frontend/src/dashboard/actions/hydrate.js | 31 ++-------
 .../src/dashboard/actions/nativeFilters.ts         |  3 +-
 .../nativeFilters/FilterBar/FilterBar.test.tsx     | 43 +++++--------
 .../FiltersConfigForm/ControlItems.tsx             | 73 +++++++++++++---------
 .../FiltersConfigForm/FiltersConfigForm.tsx        |  9 ++-
 .../nativeFilters/FiltersConfigModal/utils.ts      | 15 +++--
 .../dashboard/components/nativeFilters/utils.ts    |  3 +-
 .../dashboard/util/extractUrlParams.test.ts}       | 40 ++++++++----
 .../src/dashboard/util/extractUrlParams.ts         | 49 +++++++++++++++
 superset-frontend/src/dataMask/actions.ts          |  4 ++
 superset-frontend/src/dataMask/reducer.ts          | 41 +++++++++---
 .../src/explore/exploreUtils/index.js              |  3 +-
 .../src/filters/components/GroupBy/controlPanel.ts |  1 +
 .../src/filters/components/Select/controlPanel.ts  |  3 +
 superset/db_engine_specs/dremio.py                 | 16 ++++-
 superset/sql_lab.py                                |  2 +-
 superset/utils/core.py                             | 32 ++++++++++
 superset/views/core.py                             |  8 ++-
 superset/views/utils.py                            | 10 ++-
 .../{hana_tests.py => dremio_tests.py}             | 10 +--
 tests/utils_tests.py                               | 20 ++++++
 22 files changed, 305 insertions(+), 128 deletions(-)
 copy superset-frontend/{spec/javascripts/utils/parseCookie_spec.ts => src/dashboard/util/extractUrlParams.test.ts} (53%)
 create mode 100644 superset-frontend/src/dashboard/util/extractUrlParams.ts
 copy tests/db_engine_specs/{hana_tests.py => dremio_tests.py} (76%)

[superset] 04/06: feat(native-filters): Auto apply changes in FiltersConfigModal (#14461)

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

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

commit d0b683cb6359d0fecf1ceaae1bf241cab504fdab
Author: simcha90 <56...@users.noreply.github.com>
AuthorDate: Fri May 7 13:06:59 2021 +0300

    feat(native-filters): Auto apply changes in FiltersConfigModal (#14461)
    
    * fix:fix get permission function
    
    * feat: auto apply changes after save filters config modal
    
    * fix: repopulate default values
    
    * test: fix tests
    
    * test: fix tests
    
    * fix: fix CR notes
    
    * fix: fix CR notes
    
    (cherry picked from commit e8e838e279fe3565bc7a24545d33c808537db6f4)
---
 .../src/dashboard/actions/nativeFilters.ts         |  3 +-
 .../nativeFilters/FilterBar/FilterBar.test.tsx     | 43 +++++--------
 .../FiltersConfigForm/ControlItems.tsx             | 73 +++++++++++++---------
 .../FiltersConfigForm/FiltersConfigForm.tsx        |  9 ++-
 .../nativeFilters/FiltersConfigModal/utils.ts      | 15 +++--
 superset-frontend/src/dataMask/actions.ts          |  4 ++
 superset-frontend/src/dataMask/reducer.ts          | 41 +++++++++---
 .../src/filters/components/GroupBy/controlPanel.ts |  1 +
 .../src/filters/components/Select/controlPanel.ts  |  3 +
 9 files changed, 119 insertions(+), 73 deletions(-)

diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts
index 660f422..2afb9fe 100644
--- a/superset-frontend/src/dashboard/actions/nativeFilters.ts
+++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts
@@ -74,6 +74,7 @@ export const setFilterConfiguration = (
     filterConfig,
   });
   const { id, metadata } = getState().dashboardInfo;
+  const oldFilters = getState().nativeFilters?.filters;
 
   // TODO extract this out when makeApi supports url parameters
   const updateDashboard = makeApi<
@@ -100,7 +101,7 @@ export const setFilterConfiguration = (
       type: SET_FILTER_CONFIG_COMPLETE,
       filterConfig,
     });
-    dispatch(setDataMaskForFilterConfigComplete(filterConfig));
+    dispatch(setDataMaskForFilterConfigComplete(filterConfig, oldFilters));
   } catch (err) {
     dispatch({ type: SET_FILTER_CONFIG_FAIL, filterConfig });
     dispatch({ type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, filterConfig });
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
index afe0c0a..4507e38 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
@@ -18,7 +18,7 @@
  */
 
 import React from 'react';
-import { render, screen, cleanup } from 'spec/helpers/testing-library';
+import { render, screen } from 'spec/helpers/testing-library';
 import { Provider } from 'react-redux';
 import userEvent from '@testing-library/user-event';
 import {
@@ -71,7 +71,7 @@ const getDateControlTestId = testWithId<string>(
 const FILTER_NAME = 'Time filter 1';
 const FILTER_SET_NAME = 'New filter set';
 
-const addFilterFlow = () => {
+const addFilterFlow = async () => {
   // open filter config modal
   userEvent.click(screen.getByTestId(getTestId('collapsable')));
   userEvent.click(screen.getByTestId(getTestId('create-filter')));
@@ -80,6 +80,7 @@ const addFilterFlow = () => {
   userEvent.click(screen.getByText('Time filter'));
   userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
   userEvent.click(screen.getByText('Save'));
+  await screen.findByText('All Filters (1)');
 };
 
 const addFilterSetFlow = async () => {
@@ -175,7 +176,7 @@ describe('FilterBar', () => {
   });
 
   beforeEach(() => {
-    toggleFiltersBar.mockClear();
+    jest.clearAllMocks();
     fetchMock.get(
       'http://localhost/api/v1/time_range/?q=%27Last%20day%27',
       {
@@ -203,11 +204,6 @@ describe('FilterBar', () => {
     mockCore.makeApi = jest.fn(() => mockApi);
   });
 
-  afterEach(() => {
-    cleanup();
-    jest.clearAllMocks();
-  });
-
   const renderWrapper = (props = closedBarProps, state?: object) =>
     render(
       <Provider store={state ? getMockStore(state) : mockStore}>
@@ -306,14 +302,12 @@ describe('FilterBar', () => {
     renderWrapper(openedBarProps, stateWithoutNativeFilters);
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
 
-    addFilterFlow();
+    await addFilterFlow();
 
-    await screen.findByText('All Filters (1)');
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
   });
 
-  // TODO: fix flakiness and re-enable
-  it.skip('add and apply filter set', async () => {
+  it('add and apply filter set', async () => {
     // @ts-ignore
     global.featureFlags = {
       [FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true,
@@ -321,22 +315,17 @@ describe('FilterBar', () => {
     };
     renderWrapper(openedBarProps, stateWithoutNativeFilters);
 
-    addFilterFlow();
-
-    await screen.findByText('All Filters (1)');
+    await addFilterFlow();
 
     await addFilterSetFlow();
 
     // change filter
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-
+    userEvent.click(await screen.findByText('All Filters (1)'));
     await changeFilterValue();
     await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(2));
 
     // apply new filter value
-    await waitFor(() =>
-      expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(),
-    );
     userEvent.click(screen.getByTestId(getTestId('apply-button')));
     await waitFor(() =>
       expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(),
@@ -352,12 +341,11 @@ describe('FilterBar', () => {
     ).not.toHaveAttribute('data-selected', 'true');
     userEvent.click(screen.getByTestId(getTestId('filter-set-wrapper')));
     expect(await screen.findByText('Last week')).toBeInTheDocument();
-    expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled();
     userEvent.click(screen.getByTestId(getTestId('apply-button')));
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
   });
 
-  it.skip('add and edit filter set', async () => {
+  it('add and edit filter set', async () => {
     // @ts-ignore
     global.featureFlags = {
       [FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true,
@@ -365,9 +353,7 @@ describe('FilterBar', () => {
     };
     renderWrapper(openedBarProps, stateWithoutNativeFilters);
 
-    addFilterFlow();
-
-    await screen.findByText('All Filters (1)');
+    await addFilterFlow();
 
     await addFilterSetFlow();
 
@@ -375,12 +361,13 @@ describe('FilterBar', () => {
     userEvent.click(screen.getByText('Edit'));
 
     await changeFilterValue();
-    await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(1));
 
     // apply new changes and save them
-    expect(
-      screen.getByTestId(getTestId('filter-set-edit-save')),
-    ).toBeDisabled();
+    await waitFor(() =>
+      expect(
+        screen.getByTestId(getTestId('filter-set-edit-save')),
+      ).toBeDisabled(),
+    );
     expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled();
     userEvent.click(screen.getByTestId(getTestId('apply-button')));
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
index 82a46bf..ae20a88 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
@@ -23,13 +23,15 @@ import {
 import React, { FC } from 'react';
 import { Checkbox } from 'src/common/components';
 import { FormInstance } from 'antd/lib/form';
-import { getChartControlPanelRegistry } from '@superset-ui/core';
+import { getChartControlPanelRegistry, t } from '@superset-ui/core';
+import { Tooltip } from 'src/components/Tooltip';
 import { getControlItems, setNativeFilterFieldValues } from './utils';
 import { NativeFiltersForm, NativeFiltersFormItem } from '../types';
 import { StyledCheckboxFormItem } from './FiltersConfigForm';
 import { Filter } from '../../types';
 
 type ControlItemsProps = {
+  disabled: boolean;
   filterId: string;
   forceUpdate: Function;
   filterToEdit?: Filter;
@@ -38,6 +40,7 @@ type ControlItemsProps = {
 };
 
 const ControlItems: FC<ControlItemsProps> = ({
+  disabled,
   forceUpdate,
   form,
   filterId,
@@ -59,38 +62,48 @@ const ControlItems: FC<ControlItemsProps> = ({
             controlItem?.config?.renderTrigger,
         )
         .map(controlItem => (
-          <StyledCheckboxFormItem
-            key={controlItem.name}
-            name={['filters', filterId, 'controlValues', controlItem.name]}
-            initialValue={
-              filterToEdit?.controlValues?.[controlItem.name] ??
-              controlItem?.config?.default
+          <Tooltip
+            placement="left"
+            title={
+              controlItem.config.affectsDataMask &&
+              disabled &&
+              t('Populate "Default value" to enable this control')
             }
-            valuePropName="checked"
-            colon={false}
           >
-            <Checkbox
-              onChange={() => {
-                if (!controlItem.config.resetConfig) {
-                  forceUpdate();
-                  return;
-                }
-                setNativeFilterFieldValues(form, filterId, {
-                  defaultDataMask: null,
-                });
-                forceUpdate();
-              }}
+            <StyledCheckboxFormItem
+              key={controlItem.name}
+              name={['filters', filterId, 'controlValues', controlItem.name]}
+              initialValue={
+                filterToEdit?.controlValues?.[controlItem.name] ??
+                controlItem?.config?.default
+              }
+              valuePropName="checked"
+              colon={false}
             >
-              {controlItem.config.label}{' '}
-              {controlItem.config.description && (
-                <InfoTooltipWithTrigger
-                  placement="top"
-                  label={controlItem.config.name}
-                  tooltip={controlItem.config.description}
-                />
-              )}
-            </Checkbox>
-          </StyledCheckboxFormItem>
+              <Checkbox
+                disabled={controlItem.config.affectsDataMask && disabled}
+                onChange={() => {
+                  if (!controlItem.config.resetConfig) {
+                    forceUpdate();
+                    return;
+                  }
+                  setNativeFilterFieldValues(form, filterId, {
+                    defaultDataMask: null,
+                  });
+                  forceUpdate();
+                }}
+              >
+                {controlItem.config.label}{' '}
+                {controlItem.config.description && (
+                  <InfoTooltipWithTrigger
+                    placement="top"
+                    label={controlItem.config.name}
+                    tooltip={controlItem.config.description}
+                  />
+                )}
+              </Checkbox>
+            </StyledCheckboxFormItem>
+          </Tooltip>
         ))}
     </>
   );
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
index 182e1e0..0a3683a 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -262,6 +262,8 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
     label: filter.title,
   }));
 
+  const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset);
+
   return (
     <>
       <Typography.Title level={5}>{t('Settings')}</Typography.Title>
@@ -436,7 +438,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
           data-test="default-input"
           label={<StyledLabel>{t('Default Value')}</StyledLabel>}
         >
-          {(!hasDataset || (!isDataDirty && hasFilledDataset)) && (
+          {showDefaultValue ? (
             <DefaultValue
               setDataMask={dataMask => {
                 setNativeFilterFieldValues(form, filterId, {
@@ -449,6 +451,10 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
               form={form}
               formData={newFormData}
             />
+          ) : hasFilledDataset ? (
+            t('Click "Populate" to get "Default Value" ->')
+          ) : (
+            t('Fill all required fields to enable "Default Value"')
           )}
         </StyledFormItem>
       </StyledContainer>
@@ -463,6 +469,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
         </Checkbox>
       </StyledCheckboxFormItem>
       <ControlItems
+        disabled={!showDefaultValue}
         filterToEdit={filterToEdit}
         formFilter={formFilter}
         filterId={filterId}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
index d9ae39d..968eb17 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
@@ -42,13 +42,20 @@ export const validateForm = async (
       errors: [error],
     };
     form.setFields([fieldError]);
-    // eslint-disable-next-line no-throw-literal
-    throw { errorFields: [fieldError] };
   };
 
   try {
-    const formValues = (await form.validateFields()) as NativeFiltersForm;
-
+    let formValues: NativeFiltersForm;
+    try {
+      formValues = (await form.validateFields()) as NativeFiltersForm;
+    } catch (error) {
+      // In Jest tests in chain of tests, Ant generate `outOfDate` error so need to catch it here
+      if (!error?.errorFields?.length && error?.outOfDate) {
+        formValues = error.values;
+      } else {
+        throw error;
+      }
+    }
     const validateInstant = (filterId: string) => {
       const isInstant = formValues.filters[filterId]
         ? formValues.filters[filterId].isInstant
diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts
index 7a9bb3f..76a286c 100644
--- a/superset-frontend/src/dataMask/actions.ts
+++ b/superset-frontend/src/dataMask/actions.ts
@@ -19,6 +19,7 @@
 import { DataMask } from '@superset-ui/core';
 import { FilterConfiguration } from '../dashboard/components/nativeFilters/types';
 import { FeatureFlag, isFeatureEnabled } from '../featureFlags';
+import { Filters } from '../dashboard/reducers/types';
 
 export const UPDATE_DATA_MASK = 'UPDATE_DATA_MASK';
 export interface UpdateDataMask {
@@ -33,6 +34,7 @@ export const SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE =
 export interface SetDataMaskForFilterConfigComplete {
   type: typeof SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE;
   filterConfig: FilterConfiguration;
+  filters?: Filters;
 }
 
 export const SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL =
@@ -44,10 +46,12 @@ export interface SetDataMaskForFilterConfigFail {
 }
 export function setDataMaskForFilterConfigComplete(
   filterConfig: FilterConfiguration,
+  filters?: Filters,
 ): SetDataMaskForFilterConfigComplete {
   return {
     type: SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE,
     filterConfig,
+    filters,
   };
 }
 export function updateDataMask(
diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts
index c4ee71f..a0b0e38 100644
--- a/superset-frontend/src/dataMask/reducer.ts
+++ b/superset-frontend/src/dataMask/reducer.ts
@@ -34,6 +34,8 @@ import {
   Filter,
   FilterConfiguration,
 } from '../dashboard/components/nativeFilters/types';
+import { areObjectsEqual } from '../reduxUtils';
+import { Filters } from '../dashboard/reducers/types';
 
 export function getInitialDataMask(id?: string): DataMask;
 export function getInitialDataMask(id: string): DataMaskWithId {
@@ -54,21 +56,37 @@ export function getInitialDataMask(id: string): DataMaskWithId {
 }
 
 function fillNativeFilters(
-  data: FilterConfiguration,
-  cleanState: DataMaskStateWithId,
-  draft: DataMaskStateWithId,
+  filterConfig: FilterConfiguration,
+  mergedDataMask: DataMaskStateWithId,
+  draftDataMask: DataMaskStateWithId,
+  currentFilters?: Filters,
 ) {
-  data.forEach((filter: Filter) => {
-    cleanState[filter.id] = {
+  filterConfig.forEach((filter: Filter) => {
+    mergedDataMask[filter.id] = {
       ...getInitialDataMask(filter.id), // take initial data
       ...filter.defaultDataMask, // if something new came from BE - take it
-      ...draft[filter.id], // keep local filter data
+      ...draftDataMask[filter.id], // keep local filter data
     };
+    // if we came from filters config modal and particular filters changed take it's dataMask
+    if (
+      currentFilters &&
+      !areObjectsEqual(
+        filter.defaultDataMask,
+        currentFilters[filter.id]?.defaultDataMask,
+        { ignoreUndefined: true },
+      )
+    ) {
+      mergedDataMask[filter.id] = {
+        ...mergedDataMask[filter.id],
+        ...filter.defaultDataMask,
+      };
+    }
   });
+
   // Get back all other non-native filters
-  Object.values(draft).forEach(filter => {
+  Object.values(draftDataMask).forEach(filter => {
     if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) {
-      cleanState[filter?.id] = filter;
+      mergedDataMask[filter?.id] = filter;
     }
   });
 }
@@ -106,7 +124,12 @@ const dataMaskReducer = produce(
         );
         return cleanState;
       case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE:
-        fillNativeFilters(action.filterConfig ?? [], cleanState, draft);
+        fillNativeFilters(
+          action.filterConfig ?? [],
+          cleanState,
+          draft,
+          action.filters,
+        );
         return cleanState;
 
       default:
diff --git a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
index 33922d0..5b9b898 100644
--- a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
+++ b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
@@ -37,6 +37,7 @@ const config: ControlPanelConfig = {
               type: 'CheckboxControl',
               label: t('Multiple select'),
               default: multiSelect,
+              affectsDataMask: true,
               resetConfig: true,
               renderTrigger: true,
               description: t('Allow selecting multiple values'),
diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts
index a866c98..1fad930 100644
--- a/superset-frontend/src/filters/components/Select/controlPanel.ts
+++ b/superset-frontend/src/filters/components/Select/controlPanel.ts
@@ -61,6 +61,7 @@ const config: ControlPanelConfig = {
               label: t('Multiple select'),
               default: multiSelect,
               resetConfig: true,
+              affectsDataMask: true,
               renderTrigger: true,
               description: t('Allow selecting multiple values'),
             },
@@ -89,6 +90,7 @@ const config: ControlPanelConfig = {
               label: t('Default to first item'),
               default: defaultToFirstItem,
               resetConfig: true,
+              affectsDataMask: true,
               renderTrigger: true,
               description: t('Select first item by default'),
             },
@@ -100,6 +102,7 @@ const config: ControlPanelConfig = {
             config: {
               type: 'CheckboxControl',
               renderTrigger: true,
+              affectsDataMask: true,
               label: t('Inverse selection'),
               default: inverseSelection,
               description: t('Exclude selected values'),

[superset] 01/06: fix(annotations): pass force param to annotation request (#14483)

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

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

commit f8491dfc0a2b5dae1674afc2488c50e4362135b0
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Wed May 5 13:52:17 2021 +0300

    fix(annotations): pass force param to annotation request (#14483)
    
    * fix(annotations): pass force param to annotation request
    
    * use strtobool
    
    * add util for parsing bools
    
    (cherry picked from commit 93c7f5bb446ec6895d7702835f3157426955d5a9)
---
 superset-frontend/src/chart/chartAction.js         | 17 ++++++++++--
 .../src/explore/exploreUtils/index.js              |  3 +-
 superset/utils/core.py                             | 32 ++++++++++++++++++++++
 superset/views/core.py                             |  4 ++-
 4 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js
index 5e5fc46..9be1b6a 100644
--- a/superset-frontend/src/chart/chartAction.js
+++ b/superset-frontend/src/chart/chartAction.js
@@ -248,6 +248,7 @@ export function runAnnotationQuery(
   formData = null,
   key,
   isDashboardRequest = false,
+  force = false,
 ) {
   return function (dispatch, getState) {
     const sliceKey = key || Object.keys(getState().charts)[0];
@@ -286,7 +287,12 @@ export function runAnnotationQuery(
     }
 
     const isNative = annotation.sourceType === ANNOTATION_SOURCE_TYPES.NATIVE;
-    const url = getAnnotationJsonUrl(annotation.value, sliceFormData, isNative);
+    const url = getAnnotationJsonUrl(
+      annotation.value,
+      sliceFormData,
+      isNative,
+      force,
+    );
     const controller = new AbortController();
     const { signal } = controller;
 
@@ -462,7 +468,14 @@ export function exploreJSON(
       dispatch(updateQueryFormData(formData, key)),
       ...annotationLayers.map(x =>
         dispatch(
-          runAnnotationQuery(x, timeout, formData, key, isDashboardRequest),
+          runAnnotationQuery(
+            x,
+            timeout,
+            formData,
+            key,
+            isDashboardRequest,
+            force,
+          ),
         ),
       ),
     ]);
diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js
index 12a70d3..f1ddd17 100644
--- a/superset-frontend/src/explore/exploreUtils/index.js
+++ b/superset-frontend/src/explore/exploreUtils/index.js
@@ -57,7 +57,7 @@ export function getHostName(allowDomainSharding = false) {
   return availableDomains[currentIndex];
 }
 
-export function getAnnotationJsonUrl(slice_id, form_data, isNative) {
+export function getAnnotationJsonUrl(slice_id, form_data, isNative, force) {
   if (slice_id === null || slice_id === undefined) {
     return null;
   }
@@ -69,6 +69,7 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) {
       form_data: safeStringify(form_data, (key, value) =>
         value === null ? undefined : value,
       ),
+      force,
     })
     .toString();
 }
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 797bac5..1a711ec 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -1681,3 +1681,35 @@ def normalize_dttm_col(
         df[DTTM_ALIAS] += timedelta(hours=offset)
     if time_shift is not None:
         df[DTTM_ALIAS] += time_shift
+
+
+def parse_boolean_string(bool_str: Optional[str]) -> bool:
+    """
+    Convert a string representation of a true/false value into a boolean
+
+    >>> parse_boolean_string(None)
+    False
+    >>> parse_boolean_string('false')
+    False
+    >>> parse_boolean_string('true')
+    True
+    >>> parse_boolean_string('False')
+    False
+    >>> parse_boolean_string('True')
+    True
+    >>> parse_boolean_string('foo')
+    False
+    >>> parse_boolean_string('0')
+    False
+    >>> parse_boolean_string('1')
+    True
+
+    :param bool_str: string representation of a value that is assumed to be boolean
+    :return: parsed boolean value
+    """
+    if bool_str is None:
+        return False
+    try:
+        return bool(strtobool(bool_str.lower()))
+    except ValueError:
+        return False
diff --git a/superset/views/core.py b/superset/views/core.py
index 7fb2f47..06c03aa 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -481,6 +481,8 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
         self, layer_id: int
     ) -> FlaskResponse:
         form_data = get_form_data()[0]
+        force = utils.parse_boolean_string(request.args.get("force"))
+
         form_data["layer_id"] = layer_id
         form_data["filters"] = [{"col": "layer_id", "op": "==", "val": layer_id}]
         # Set all_columns to ensure the TableViz returns the necessary columns to the
@@ -499,7 +501,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
             "changed_by_fk",
         ]
         datasource = AnnotationDatasource()
-        viz_obj = viz.viz_types["table"](datasource, form_data=form_data, force=False)
+        viz_obj = viz.viz_types["table"](datasource, form_data=form_data, force=force)
         payload = viz_obj.get_payload()
         return data_payload_response(*viz_obj.payload_json_and_has_error(payload))
 

[superset] 02/06: fix: SQL Statement on QUERY_LOGGER prints none to log (#14358)

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

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

commit 099514d3c0a31d5966de18fa21d2dc7c4e6c7512
Author: cccs-rc <62...@users.noreply.github.com>
AuthorDate: Thu May 6 10:50:19 2021 -0400

    fix: SQL Statement on QUERY_LOGGER prints none to log (#14358)
    
    (cherry picked from commit f55882e75606c107b49a270c74bb620e9d02cef6)
---
 superset/sql_lab.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/sql_lab.py b/superset/sql_lab.py
index 9b62f9c..cd8818e 100644
--- a/superset/sql_lab.py
+++ b/superset/sql_lab.py
@@ -228,6 +228,7 @@ def execute_sql_statement(
     # Hook to allow environment-specific mutation (usually comments) to the SQL
     sql = SQL_QUERY_MUTATOR(sql, user_name, security_manager, database)
     try:
+        query.executed_sql = sql
         if log_query:
             log_query(
                 query.database.sqlalchemy_uri,
@@ -238,7 +239,6 @@ def execute_sql_statement(
                 security_manager,
                 log_params,
             )
-        query.executed_sql = sql
         session.commit()
         with stats_timing("sqllab.query.time_executing_query", stats_logger):
             logger.debug("Query %d: Running query: %s", query.id, sql)

[superset] 05/06: feat(dremio): implement convert_dttm method (#14519)

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

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

commit d0a67065272a0b5d2715a3b28d67321eb5fc22d4
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Fri May 7 15:31:58 2021 +0300

    feat(dremio): implement convert_dttm method (#14519)
    
    (cherry picked from commit d1d98d81b03035f1047326502151ab70f8a6b43e)
---
 superset/db_engine_specs/dremio.py                 | 16 ++++++++++-
 .../db_engine_specs/dremio_tests.py                | 32 +++++++++-------------
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/superset/db_engine_specs/dremio.py b/superset/db_engine_specs/dremio.py
index 4a11424..a76909b 100644
--- a/superset/db_engine_specs/dremio.py
+++ b/superset/db_engine_specs/dremio.py
@@ -14,10 +14,14 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from datetime import datetime
+from typing import Optional
+
 from superset.db_engine_specs.base import BaseEngineSpec
+from superset.utils import core as utils
 
 
-class DremioBaseEngineSpec(BaseEngineSpec):
+class DremioEngineSpec(BaseEngineSpec):
 
     engine = "dremio"
     engine_name = "Dremio"
@@ -37,3 +41,13 @@ class DremioBaseEngineSpec(BaseEngineSpec):
     @classmethod
     def epoch_to_dttm(cls) -> str:
         return "TO_DATE({col})"
+
+    @classmethod
+    def convert_dttm(cls, target_type: str, dttm: datetime) -> Optional[str]:
+        tt = target_type.upper()
+        if tt == utils.TemporalType.DATE:
+            return f"TO_DATE('{dttm.date().isoformat()}', 'YYYY-MM-DD')"
+        if tt == utils.TemporalType.TIMESTAMP:
+            dttm_formatted = dttm.isoformat(sep=" ", timespec="milliseconds")
+            return f"""TO_TIMESTAMP('{dttm_formatted}', 'YYYY-MM-DD HH24:MI:SS.FFF')"""
+        return None
diff --git a/superset/db_engine_specs/dremio.py b/tests/db_engine_specs/dremio_tests.py
similarity index 54%
copy from superset/db_engine_specs/dremio.py
copy to tests/db_engine_specs/dremio_tests.py
index 4a11424..02d21c6 100644
--- a/superset/db_engine_specs/dremio.py
+++ b/tests/db_engine_specs/dremio_tests.py
@@ -14,26 +14,20 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from superset.db_engine_specs.base import BaseEngineSpec
+from superset.db_engine_specs.dremio import DremioEngineSpec
+from tests.db_engine_specs.base_tests import TestDbEngineSpec
 
 
-class DremioBaseEngineSpec(BaseEngineSpec):
+class TestDremioDbEngineSpec(TestDbEngineSpec):
+    def test_convert_dttm(self):
+        dttm = self.get_dttm()
 
-    engine = "dremio"
-    engine_name = "Dremio"
+        self.assertEqual(
+            DremioEngineSpec.convert_dttm("DATE", dttm),
+            "TO_DATE('2019-01-02', 'YYYY-MM-DD')",
+        )
 
-    _time_grain_expressions = {
-        None: "{col}",
-        "PT1S": "DATE_TRUNC('second', {col})",
-        "PT1M": "DATE_TRUNC('minute', {col})",
-        "PT1H": "DATE_TRUNC('hour', {col})",
-        "P1D": "DATE_TRUNC('day', {col})",
-        "P1W": "DATE_TRUNC('week', {col})",
-        "P1M": "DATE_TRUNC('month', {col})",
-        "P0.25Y": "DATE_TRUNC('quarter', {col})",
-        "P1Y": "DATE_TRUNC('year', {col})",
-    }
-
-    @classmethod
-    def epoch_to_dttm(cls) -> str:
-        return "TO_DATE({col})"
+        self.assertEqual(
+            DremioEngineSpec.convert_dttm("TIMESTAMP", dttm),
+            "TO_TIMESTAMP('2019-01-02 03:04:05.678', 'YYYY-MM-DD HH24:MI:SS.FFF')",
+        )

[superset] 06/06: fix(chart-data): handle url_params in csv export and native filters (#14526)

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

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

commit f2236fcb8ff5bee075dbdadf82aaf61f863f7c78
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Fri May 7 21:07:44 2021 +0300

    fix(chart-data): handle url_params in csv export and native filters (#14526)
    
    (cherry picked from commit 66a4c94a1ed542e69fe6399bab4c01d4540486cf)
---
 superset-frontend/src/dashboard/actions/hydrate.js | 31 ++-----------
 .../dashboard/components/nativeFilters/utils.ts    |  3 +-
 .../src/dashboard/util/extractUrlParams.test.ts    | 53 ++++++++++++++++++++++
 .../src/dashboard/util/extractUrlParams.ts         | 49 ++++++++++++++++++++
 superset/views/utils.py                            | 10 +++-
 tests/utils_tests.py                               | 20 ++++++++
 6 files changed, 137 insertions(+), 29 deletions(-)

diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js
index 9e4e2b0..64eaeca 100644
--- a/superset-frontend/src/dashboard/actions/hydrate.js
+++ b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -24,7 +24,6 @@ import {
   CategoricalColorNamespace,
   getChartMetadataRegistry,
 } from '@superset-ui/core';
-import querystring from 'query-string';
 
 import { chart } from 'src/chart/chartReducer';
 import { initSliceEntities } from 'src/dashboard/reducers/sliceEntities';
@@ -55,27 +54,7 @@ import getLocationHash from 'src/dashboard/util/getLocationHash';
 import newComponentFactory from 'src/dashboard/util/newComponentFactory';
 import { TIME_RANGE } from 'src/visualizations/FilterBox/FilterBox';
 import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
-
-const reservedQueryParams = new Set(['standalone', 'edit']);
-
-/**
- * Returns the url params that are used to customize queries
- * in datasets built using sql lab.
- * We may want to extract this to some kind of util in the future.
- */
-const extractUrlParams = queryParams =>
-  Object.entries(queryParams).reduce((acc, [key, value]) => {
-    if (reservedQueryParams.has(key)) return acc;
-    // if multiple url params share the same key (?foo=bar&foo=baz), they will appear as an array.
-    // Only one value can be used for a given query param, so we just take the first one.
-    if (Array.isArray(value)) {
-      return {
-        ...acc,
-        [key]: value[0],
-      };
-    }
-    return { ...acc, [key]: value };
-  }, {});
+import extractUrlParams from '../util/extractUrlParams';
 
 export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';
 
@@ -85,9 +64,9 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => (
 ) => {
   const { user, common } = getState();
   let { metadata } = dashboardData;
-  const queryParams = querystring.parse(window.location.search);
-  const urlParams = extractUrlParams(queryParams);
-  const editMode = queryParams.edit === 'true';
+  const regularUrlParams = extractUrlParams('regular');
+  const reservedUrlParams = extractUrlParams('reserved');
+  const editMode = reservedUrlParams.edit === 'true';
 
   let preselectFilters = {};
 
@@ -154,7 +133,7 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => (
       ...slice.form_data,
       url_params: {
         ...slice.form_data.url_params,
-        ...urlParams,
+        ...regularUrlParams,
       },
     };
     chartQueries[key] = {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index a42540a..a674a27 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -28,6 +28,7 @@ import {
 import { Charts } from 'src/dashboard/types';
 import { RefObject } from 'react';
 import { DataMaskStateWithId } from 'src/dataMask/types';
+import extractUrlParams from 'src/dashboard/util/extractUrlParams';
 import { Filter } from './types';
 
 export const getFormData = ({
@@ -76,7 +77,7 @@ export const getFormData = ({
     defaultValue: defaultDataMask?.filterState?.value,
     time_range,
     time_range_endpoints: ['inclusive', 'exclusive'],
-    url_params: {},
+    url_params: extractUrlParams('regular'),
     viz_type: filterType,
     inputRef,
   };
diff --git a/superset-frontend/src/dashboard/util/extractUrlParams.test.ts b/superset-frontend/src/dashboard/util/extractUrlParams.test.ts
new file mode 100644
index 0000000..dd81c10
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/extractUrlParams.test.ts
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 extractUrlParams from './extractUrlParams';
+
+const originalWindowLocation = window.location;
+
+describe('extractUrlParams', () => {
+  beforeAll(() => {
+    // @ts-ignore
+    delete window.location;
+    // @ts-ignore
+    window.location = { search: '?edit=true&abc=123' };
+  });
+
+  afterAll(() => {
+    window.location = originalWindowLocation;
+  });
+
+  it('returns all urlParams', () => {
+    expect(extractUrlParams('all')).toEqual({
+      edit: 'true',
+      abc: '123',
+    });
+  });
+
+  it('returns reserved urlParams', () => {
+    expect(extractUrlParams('reserved')).toEqual({
+      edit: 'true',
+    });
+  });
+
+  it('returns regular urlParams', () => {
+    expect(extractUrlParams('regular')).toEqual({
+      abc: '123',
+    });
+  });
+});
diff --git a/superset-frontend/src/dashboard/util/extractUrlParams.ts b/superset-frontend/src/dashboard/util/extractUrlParams.ts
new file mode 100644
index 0000000..7ef7dd9
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/extractUrlParams.ts
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 querystring from 'query-string';
+import { JsonObject } from '@superset-ui/core';
+
+const reservedQueryParams = new Set(['standalone', 'edit']);
+
+export type UrlParamType = 'reserved' | 'regular' | 'all';
+
+/**
+ * Returns the url params that are used to customize queries
+ */
+export default function extractUrlParams(
+  urlParamType: UrlParamType,
+): JsonObject {
+  const queryParams = querystring.parse(window.location.search);
+  return Object.entries(queryParams).reduce((acc, [key, value]) => {
+    if (
+      (urlParamType === 'regular' && reservedQueryParams.has(key)) ||
+      (urlParamType === 'reserved' && !reservedQueryParams.has(key))
+    )
+      return acc;
+    // if multiple url params share the same key (?foo=bar&foo=baz), they will appear as an array.
+    // Only one value can be used for a given query param, so we just take the first one.
+    if (Array.isArray(value)) {
+      return {
+        ...acc,
+        [key]: value[0],
+      };
+    }
+    return { ...acc, [key]: value };
+  }, {});
+}
diff --git a/superset/views/utils.py b/superset/views/utils.py
index f45aa04..3c9021c 100644
--- a/superset/views/utils.py
+++ b/superset/views/utils.py
@@ -129,7 +129,7 @@ def loads_request_json(request_json_data: str) -> Dict[Any, Any]:
         return {}
 
 
-def get_form_data(
+def get_form_data(  # pylint: disable=too-many-locals
     slice_id: Optional[int] = None, use_slice_data: bool = False
 ) -> Tuple[Dict[str, Any], Optional[Slice]]:
     form_data = {}
@@ -144,7 +144,13 @@ def get_form_data(
     if request_json_data:
         form_data.update(request_json_data)
     if request_form_data:
-        form_data.update(loads_request_json(request_form_data))
+        parsed_form_data = loads_request_json(request_form_data)
+        # some chart data api requests are form_data
+        queries = parsed_form_data.get("queries")
+        if isinstance(queries, list):
+            form_data.update(queries[0])
+        else:
+            form_data.update(parsed_form_data)
     # request params can overwrite the body
     if request_args_data:
         form_data.update(loads_request_json(request_args_data))
diff --git a/tests/utils_tests.py b/tests/utils_tests.py
index c054624..5781965 100644
--- a/tests/utils_tests.py
+++ b/tests/utils_tests.py
@@ -1015,6 +1015,26 @@ class TestUtils(SupersetTestCase):
 
             self.assertEqual(slc, None)
 
+    def test_get_form_data_request_form_with_queries(self) -> None:
+        # the CSV export uses for requests, even when sending requests to
+        # /api/v1/chart/data
+        with app.test_request_context(
+            data={
+                "form_data": json.dumps({"queries": [{"url_params": {"foo": "bar"}}]})
+            }
+        ):
+            form_data, slc = get_form_data()
+
+            self.assertEqual(
+                form_data,
+                {
+                    "url_params": {"foo": "bar"},
+                    "time_range_endpoints": get_time_range_endpoints(form_data={}),
+                },
+            )
+
+            self.assertEqual(slc, None)
+
     def test_get_form_data_request_args_and_form(self) -> None:
         with app.test_request_context(
             data={"form_data": json.dumps({"foo": "bar"})},

[superset] 03/06: fix: parameterize titles correctly (#14509)

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

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

commit c2d24782d974863afa8ffb7a0ca4c8f2b10bb849
Author: David Aaron Suddjian <18...@users.noreply.github.com>
AuthorDate: Thu May 6 15:30:49 2021 -0700

    fix: parameterize titles correctly (#14509)
    
    (cherry picked from commit 52dbe311c71e2f188c7b961f092eec5459f3bfab)
---
 superset/views/core.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/superset/views/core.py b/superset/views/core.py
index 06c03aa..5ee2ed8 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -824,7 +824,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
                 bootstrap_data, default=utils.pessimistic_json_iso_dttm_ser
             ),
             entry="explore",
-            title=title,
+            title=title.__str__(),
             standalone_mode=standalone_mode,
         )
 
@@ -2821,7 +2821,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
 
         return self.render_template(
             "superset/basic.html",
-            title=_("%(user)s's profile", user=username),
+            title=_("%(user)s's profile", user=username).__str__(),
             entry="profile",
             bootstrap_data=json.dumps(
                 payload, default=utils.pessimistic_json_iso_dttm_ser