You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yo...@apache.org on 2021/01/23 05:25:43 UTC

[superset] branch master updated: fix(native-filters): Reset column field for removed dataset (#12519)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 0c38134  fix(native-filters): Reset column field for removed dataset (#12519)
0c38134 is described below

commit 0c38134e5cdf62788721077379179a4f0475e366
Author: Agata Stawarz <47...@users.noreply.github.com>
AuthorDate: Sat Jan 23 06:24:31 2021 +0100

    fix(native-filters): Reset column field for removed dataset (#12519)
    
    * Reset column field for removed dataset
    
    * Fix issue with cleaning exisiting field
    
    * Fix lint error and remove unnecessary import
    
    * Change message text for wrong filter
    
    * Add Basic Error Alert component
    
    * Fix hiding error after filter correction
---
 .../components/ErrorMessage/BasicErrorAlert.tsx    | 69 ++++++++++++++++++++++
 .../src/components/SupersetResourceSelect.tsx      | 16 +++--
 .../components/nativeFilters/ColumnSelect.tsx      | 12 +++-
 .../components/nativeFilters/FilterBar.tsx         | 28 +++++++--
 .../components/nativeFilters/FilterConfigForm.tsx  |  6 +-
 5 files changed, 116 insertions(+), 15 deletions(-)

diff --git a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx
new file mode 100644
index 0000000..b66bebf
--- /dev/null
+++ b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx
@@ -0,0 +1,69 @@
+/**
+ * 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 React from 'react';
+import { styled, supersetTheme } from '@superset-ui/core';
+import Icon from '../Icon';
+import { ErrorLevel } from './types';
+
+const StyledContainer = styled.div<{ level: ErrorLevel }>`
+  display: flex;
+  flex-direction: row;
+  background-color: ${({ level, theme }) => theme.colors[level].light2};
+  border-radius: ${({ theme }) => theme.borderRadius}px;
+  border: 1px solid ${({ level, theme }) => theme.colors[level].base};
+  color: ${({ level, theme }) => theme.colors[level].dark2};
+  padding: ${({ theme }) => theme.gridUnit * 2}px;
+  margin-bottom: ${({ theme }) => theme.gridUnit}px;
+  width: 100%;
+`;
+
+const StyledContent = styled.div`
+  display: flex;
+  flex-direction: column;
+  margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+`;
+
+const StyledTitle = styled.span`
+  font-weight: ${({ theme }) => theme.typography.weights.bold};
+`;
+
+interface BasicErrorAlertProps {
+  title: string;
+  body: string;
+  level: ErrorLevel;
+}
+
+export default function BasicErrorAlert({
+  body,
+  level,
+  title,
+}: BasicErrorAlertProps) {
+  return (
+    <StyledContainer level={level}>
+      <Icon
+        name={level === 'error' ? 'error-solid' : 'warning-solid'}
+        color={supersetTheme.colors[level].base}
+      />
+      <StyledContent>
+        <StyledTitle>{title}</StyledTitle>
+        <p>{body}</p>
+      </StyledContent>
+    </StyledContainer>
+  );
+}
diff --git a/superset-frontend/src/components/SupersetResourceSelect.tsx b/superset-frontend/src/components/SupersetResourceSelect.tsx
index 56ae43b..3f69885 100644
--- a/superset-frontend/src/components/SupersetResourceSelect.tsx
+++ b/superset-frontend/src/components/SupersetResourceSelect.tsx
@@ -31,7 +31,7 @@ export type Value<V> = { value: V; label: string };
 export interface SupersetResourceSelectProps<T = unknown, V = string> {
   value?: Value<V> | null;
   initialId?: number | string;
-  onChange?: (value: Value<V>) => void;
+  onChange?: (value?: Value<V>) => void;
   isMulti?: boolean;
   searchColumn?: string;
   resource?: string; // e.g. "dataset", "dashboard/related/owners"
@@ -74,11 +74,15 @@ export default function SupersetResourceSelect<T, V>({
     if (initialId == null) return;
     cachedSupersetGet({
       endpoint: `/api/v1/${resource}/${initialId}`,
-    }).then(response => {
-      const { result } = response.json;
-      const value = transformItem ? transformItem(result) : result;
-      if (onChange) onChange(value);
-    });
+    })
+      .then(response => {
+        const { result } = response.json;
+        const value = transformItem ? transformItem(result) : result;
+        if (onChange) onChange(value);
+      })
+      .catch(response => {
+        if (response?.status === 404 && onChange) onChange(undefined);
+      });
   }, [resource, initialId]); // eslint-disable-line react-hooks/exhaustive-deps
 
   function loadOptions(input: string) {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/ColumnSelect.tsx b/superset-frontend/src/dashboard/components/nativeFilters/ColumnSelect.tsx
index 2c83c4a..22bd15c 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/ColumnSelect.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/ColumnSelect.tsx
@@ -62,6 +62,7 @@ export function ColumnSelect({
       { name: ['filters', filterId, 'column'], touched: false, value: null },
     ]);
   }, [form, filterId]);
+
   useChangeEffect(datasetId, previous => {
     if (previous != null) {
       resetColumnField();
@@ -73,10 +74,15 @@ export function ColumnSelect({
     return cachedSupersetGet({
       endpoint: `/api/v1/dataset/${datasetId}`,
     }).then(
-      ({ json: { result } }) =>
-        result.columns
+      ({ json: { result } }) => {
+        const columns = result.columns
           .map((col: any) => col.column_name)
-          .sort((a: string, b: string) => a.localeCompare(b)),
+          .sort((a: string, b: string) => a.localeCompare(b));
+        if (!columns.includes(value)) {
+          resetColumnField();
+        }
+        return columns;
+      },
       async badResponse => {
         const { error, message } = await getClientErrorObject(badResponse);
         let errorText = message || error || t('An error has occurred');
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx
index 0b45d95..a81e5d8 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx
@@ -32,6 +32,7 @@ import Icon from 'src/components/Icon';
 import { getChartDataRequest } from 'src/chart/chartAction';
 import { areObjectsEqual } from 'src/reduxUtils';
 import Loading from 'src/components/Loading';
+import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
 import FilterConfigurationLink from './FilterConfigurationLink';
 // import FilterScopeModal from 'src/dashboard/components/filterscope/FilterScopeModal';
 
@@ -216,6 +217,7 @@ const FilterValue: React.FC<FilterProps> = ({
   const cascadingFilters = useCascadingFilters(id);
   const [loading, setLoading] = useState<boolean>(true);
   const [state, setState] = useState([]);
+  const [error, setError] = useState<boolean>(false);
   const [formData, setFormData] = useState<Partial<QueryFormData>>({});
   const [target] = targets;
   const { datasetId = 18, column } = target;
@@ -248,12 +250,18 @@ const FilterValue: React.FC<FilterProps> = ({
         formData: newFormData,
         force: false,
         requestParams: { dashboardId: 0 },
-      }).then(response => {
-        setState(response.result);
-        setLoading(false);
-      });
+      })
+        .then(response => {
+          setState(response.result);
+          setError(false);
+          setLoading(false);
+        })
+        .catch(() => {
+          setError(true);
+          setLoading(false);
+        });
     }
-  }, [cascadingFilters]);
+  }, [cascadingFilters, datasetId, groupby]);
 
   const setExtraFormData = (extraFormData: ExtraFormData) =>
     onExtraFormDataChange(filter, extraFormData);
@@ -266,6 +274,16 @@ const FilterValue: React.FC<FilterProps> = ({
     );
   }
 
+  if (error) {
+    return (
+      <BasicErrorAlert
+        title={t('Cannot load filter')}
+        body={t('Check configuration')}
+        level="error"
+      />
+    );
+  }
+
   return (
     <Form
       onFinish={values => {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx
index 0608483..9489209 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx
@@ -98,7 +98,11 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
   form,
   parentFilters,
 }) => {
-  const [dataset, setDataset] = useState<Value<number> | undefined>();
+  const [dataset, setDataset] = useState<Value<number> | undefined>(
+    filterToEdit?.targets[0].datasetId
+      ? { label: '', value: filterToEdit?.targets[0].datasetId }
+      : undefined,
+  );
 
   const onDatasetSelectError = useCallback(
     ({ error, message }: ClientErrorObject) => {