Merge branch 'master' of https://github.com/apache/superset into fix/…

…issue_16962_dup_d3category10

.github/workflows/superset-frontend.yml

@@ -28,6 +28,7 @@ jobs:
         continue-on-error: true
         run: ./scripts/ci_check_no_file_changes.sh frontend
       - name: Setup Node.js
+        if: steps.check.outcome == 'failure'
         uses: actions/setup-node@v2
         with:
           node-version:  '16'

.github/workflows/superset-python-misc.yml

@@ -29,6 +29,7 @@ jobs:
         continue-on-error: true
         run: ./scripts/ci_check_no_file_changes.sh python
       - name: Setup Python
+        if: steps.check.outcome == 'failure'
         uses: actions/setup-python@v2
         with:
           python-version: ${{ matrix.python-version }}

CONTRIBUTING.md

@@ -650,18 +650,23 @@ pre-commit run --all-files
 
 ## Linting
 
-Lint the project with:
+### Python
+
+We use [Pylint](https://pylint.org/) for linting which can be invoked via:
 
 ```bash
 # for python
 tox -e pylint
+```
 
-Alternatively, you can use pre-commit (mentioned above) for python linting
+In terms of best practices please advoid blanket disablement of Pylint messages globally (via `.pylintrc`) or top-level within the file header, albeit there being a few exceptions. Disablement should occur inline as it prevents masking issues and provides context as to why said message is disabled.
 
-The Python code is auto-formatted using [Black](https://github.com/python/black) which
+Additionally the Python code is auto-formatted using [Black](https://github.com/python/black) which
 is configured as a pre-commit hook. There are also numerous [editor integrations](https://black.readthedocs.io/en/stable/editor_integration.html)
 
-# for frontend
+### TypeScript
+
+```bash
 cd superset-frontend
 npm ci
 npm run lint

superset-frontend/src/components/DatabaseSelector/index.tsx

@@ -251,6 +251,7 @@ export default function DatabaseSelector({
     return renderSelectRow(
       <Select
         ariaLabel={t('Select database or type database name')}
+        optionFilterProps={['database_name', 'value']}
         data-test="select-database"
         header={<FormLabel>{t('Database')}</FormLabel>}
         lazyLoading={false}

superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx

@@ -58,8 +58,8 @@ export default function BasicErrorAlert({
 
   return (
     <StyledContainer level={level} role="alert">
-      {level === 'error' ? (
-        <Icons.ErrorSolid iconColor={theme.colors[level].base} />
+      {!level || level === 'error' ? (
+        <Icons.ErrorSolid iconColor={theme.colors.error.base} />
       ) : (
         <Icons.WarningSolid iconColor={theme.colors[level].base} />
       )}

superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx

@@ -108,10 +108,10 @@ export default function ErrorAlert({
     <ErrorAlertDiv level={level} role="alert">
       <div className="top-row">
         <LeftSideContent>
-          {level === 'error' ? (
+          {!level || level === 'error' ? (
             <Icons.ErrorSolid
               className="icon"
-              iconColor={theme.colors[level].base}
+              iconColor={theme.colors.error.base}
             />
           ) : (
             <Icons.WarningSolid
@@ -171,10 +171,10 @@ export default function ErrorAlert({
           onHide={() => setIsModalOpen(false)}
           title={
             <div className="header">
-              {level === 'error' ? (
+              {!level || level === 'error' ? (
                 <Icons.ErrorSolid
                   className="icon"
-                  iconColor={theme.colors[level].base}
+                  iconColor={theme.colors.error.base}
                 />
               ) : (
                 <Icons.WarningSolid

superset-frontend/src/components/Select/Select.tsx

@@ -181,6 +181,7 @@ const Select = ({
   mode = 'single',
   name,
   notFoundContent,
+  onError,
   onChange,
   onClear,
   optionFilterProps = ['label', 'value'],
@@ -328,15 +329,18 @@ const Select = ({
     setSearchedValue('');
   };
 
-  const onError = (response: Response) =>
-    getClientErrorObject(response).then(e => {
-      const { error } = e;
-      setError(error);
+  const internalOnError = useCallback(
+    (response: Response) =>
+      getClientErrorObject(response).then(e => {
+        const { error } = e;
+        setError(error);
 
-      if (props.onError) {
-        props.onError(error);
-      }
-    });
+        if (onError) {
+          onError(error);
+        }
+      }),
+    [onError],
+  );
 
   const handleData = (data: OptionsType) => {
     let mergedData: OptionsType = [];
@@ -391,13 +395,13 @@ const Select = ({
             setAllValuesLoaded(true);
           }
         })
-        .catch(onError)
+        .catch(internalOnError)
         .finally(() => {
           setIsLoading(false);
           setIsTyping(false);
         });
     },
-    [allValuesLoaded, fetchOnlyOnSearch, options],
+    [allValuesLoaded, fetchOnlyOnSearch, internalOnError, options],
   );
 
   const handleOnSearch = useMemo(

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx

@@ -63,7 +63,8 @@ const FilterControls: FC<FilterControlsProps> = ({
       dataMask: dataMaskSelected[filter.id],
     }));
     return buildCascadeFiltersTree(filtersWithValue);
-  }, [filterValues, dataMaskSelected]);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [JSON.stringify(filterValues), dataMaskSelected]);
   const cascadeFilterIds = new Set(cascadeFilters.map(item => item.id));
 
   const [filtersInScope, filtersOutOfScope] = useSelectFiltersInScope(

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx

@@ -29,7 +29,7 @@ import {
   getChartMetadataRegistry,
 } from '@superset-ui/core';
 import { useDispatch, useSelector } from 'react-redux';
-import { areObjectsEqual } from 'src/reduxUtils';
+import { isEqual, isEqualWith } from 'lodash';
 import { getChartDataRequest } from 'src/chart/chartAction';
 import Loading from 'src/components/Loading';
 import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
@@ -105,10 +105,17 @@ const FilterValue: React.FC<FilterProps> = ({
       time_range,
     });
     const filterOwnState = filter.dataMask?.ownState || {};
+    // TODO: We should try to improve our useEffect hooks to depend more on
+    // granular information instead of big objects that require deep comparison.
+    const customizer = (
+      objValue: Partial<QueryFormData>,
+      othValue: Partial<QueryFormData>,
+      key: string,
+    ) => (key === 'url_params' ? true : undefined);
     if (
       !isRefreshing &&
-      (!areObjectsEqual(formData, newFormData) ||
-        !areObjectsEqual(ownState, filterOwnState) ||
+      (!isEqualWith(formData, newFormData, customizer) ||
+        !isEqual(ownState, filterOwnState) ||
         isDashboardRefreshing)
     ) {
       setFormData(newFormData);

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -180,7 +180,6 @@ type DBReducerActionType =
         database_name?: string;
         engine?: string;
         configuration_method: CONFIGURATION_METHOD;
-        paramProperties?: Record<string, any>;
       };
     }
   | {
@@ -651,7 +650,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
           configuration_method: isDynamic
             ? CONFIGURATION_METHOD.DYNAMIC_FORM
             : CONFIGURATION_METHOD.SQLALCHEMY_URI,
-          paramProperties: parameters?.properties,
         },
       });
     }