Merge branch 'master' into john-bodley--fix-native-filters-modal-caching

.pre-commit-config.yaml

@@ -20,16 +20,17 @@ repos:
     hooks:
       - id: isort
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.941
+    rev: v1.0.1
     hooks:
       - id: mypy
+        args: [--check-untyped-defs]
         additional_dependencies: [types-all]
   - repo: https://github.com/peterdemin/pip-compile-multi
-    rev: v2.4.1
+    rev: v2.6.2
     hooks:
       - id: pip-compile-multi-verify
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v3.2.0
+    rev: v4.4.0
     hooks:
       - id: check-docstring-first
       - id: check-added-large-files
@@ -41,7 +42,7 @@ repos:
       - id: trailing-whitespace
         args: ["--markdown-linebreak-ext=md"]
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 23.1.0
     hooks:
       - id: black
         language_version: python3

RELEASING/changelog.py

@@ -138,7 +138,7 @@ def _get_pull_request_details(self, git_log: GitLog) -> Dict[str, Any]:
         title = pr_info.title if pr_info else git_log.message
         pr_type = re.match(SUPERSET_PULL_REQUEST_TYPES, title)
         if pr_type:
-            pr_type = pr_type.group().strip('"')
+            pr_type = pr_type.group().strip('"')  # type: ignore
 
         labels = (" | ").join([label.name for label in pr_info.labels])
         is_risky = self._is_risk_pull_request(pr_info.labels)

superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx

@@ -59,11 +59,9 @@ export function ColumnOption({
       <Tooltip id="metric-name-tooltip" title={tooltipText}>
         <span
           className="option-label column-option-label"
-          css={(theme: SupersetTheme) =>
-            css`
-              margin-right: ${theme.gridUnit}px;
-            `
-          }
+          css={(theme: SupersetTheme) => css`
+            margin-right: ${theme.gridUnit}px;
+          `}
           ref={labelRef}
         >
           {getColumnLabelText(column)}

superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx

@@ -71,11 +71,9 @@ export function MetricOption({
   const label = (
     <span
       className="option-label metric-option-label"
-      css={(theme: SupersetTheme) =>
-        css`
-          margin-right: ${theme.gridUnit}px;
-        `
-      }
+      css={(theme: SupersetTheme) => css`
+        margin-right: ${theme.gridUnit}px;
+      `}
       ref={labelRef}
     >
       {link}

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts

@@ -25,12 +25,13 @@ import {
 export function extractExtraMetrics(
   formData: QueryFormData,
 ): QueryFormMetric[] {
-  const { groupby, timeseries_limit_metric, x_axis_sort } = formData;
+  const { groupby, timeseries_limit_metric, x_axis_sort, metrics } = formData;
   const extra_metrics: QueryFormMetric[] = [];
   if (
     !(groupby || []).length &&
     timeseries_limit_metric &&
-    getMetricLabel(timeseries_limit_metric) === x_axis_sort
+    getMetricLabel(timeseries_limit_metric) === x_axis_sort &&
+    !metrics?.some(metric => getMetricLabel(metric) === x_axis_sort)
   ) {
     extra_metrics.push(timeseries_limit_metric);
   }

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx

@@ -79,6 +79,7 @@ export const xAxisSortControl = {
         ...ensureIsArray(controls?.metrics?.value as QueryFormMetric),
         controls?.timeseries_limit_metric?.value as QueryFormMetric,
       ].filter(Boolean);
+      const metricLabels = [...new Set(metrics.map(getMetricLabel))];
       const options = [
         ...columns.map(column => {
           const value = getColumnLabel(column);
@@ -87,13 +88,10 @@ export const xAxisSortControl = {
             label: dataset?.verbose_map?.[value] || value,
           };
         }),
-        ...metrics.map(metric => {
-          const value = getMetricLabel(metric);
-          return {
-            value,
-            label: dataset?.verbose_map?.[value] || value,
-          };
-        }),
+        ...metricLabels.map(value => ({
+          value,
+          label: dataset?.verbose_map?.[value] || value,
+        })),
       ];
 
       const shouldReset = !(

superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts

@@ -92,3 +92,35 @@ test('returns empty array if groupby populated', () => {
     }),
   ).toEqual([]);
 });
+
+test('returns empty array if timeseries_limit_metric and x_axis_sort are included in main metrics array', () => {
+  expect(
+    extractExtraMetrics({
+      ...baseFormData,
+      timeseries_limit_metric: 'a',
+      x_axis_sort: 'a',
+    }),
+  ).toEqual([]);
+});
+
+test('returns empty array if timeseries_limit_metric and x_axis_sort are included in main metrics array with adhoc metrics', () => {
+  expect(
+    extractExtraMetrics({
+      ...baseFormData,
+      metrics: [
+        'a',
+        {
+          expressionType: 'SIMPLE',
+          aggregate: 'SUM',
+          column: { column_name: 'num' },
+        },
+      ],
+      timeseries_limit_metric: {
+        expressionType: 'SIMPLE',
+        aggregate: 'SUM',
+        column: { column_name: 'num' },
+      },
+      x_axis_sort: 'SUM(num)',
+    }),
+  ).toEqual([]);
+});

superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx

@@ -91,10 +91,10 @@ test('table should be visible when expanded is true', async () => {
   const { container } = await renderAndWait(mockedProps, store);
 
   const dbSelect = screen.getByRole('combobox', {
-    name: 'Select database or type database name',
+    name: 'Select database or type to search databases',
   });
   const schemaSelect = screen.getByRole('combobox', {
-    name: 'Select schema or type schema name',
+    name: 'Select schema or type to search schemas',
   });
   const dropdown = screen.getByText(/Table/i);
   const abUser = screen.queryAllByText(/ab_user/i);

superset-frontend/src/SqlLab/constants.ts

@@ -83,6 +83,7 @@ export const BYTES_PER_CHAR = 2;
 // browser's localStorage max usage constants
 export const LOCALSTORAGE_MAX_QUERY_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours
 export const LOCALSTORAGE_MAX_USAGE_KB = 5 * 1024; // 5M
+export const LOCALSTORAGE_MAX_QUERY_RESULTS_KB = 1 * 1024; // 1M
 export const LOCALSTORAGE_WARNING_THRESHOLD = 0.9;
 export const LOCALSTORAGE_WARNING_MESSAGE_THROTTLE_MS = 8000; // danger type toast duration
 

superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js

@@ -20,7 +20,12 @@ import {
   emptyQueryResults,
   clearQueryEditors,
 } from 'src/SqlLab/utils/reduxStateToLocalStorageHelper';
-import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from 'src/SqlLab/constants';
+import {
+  KB_STORAGE,
+  BYTES_PER_CHAR,
+  LOCALSTORAGE_MAX_QUERY_AGE_MS,
+  LOCALSTORAGE_MAX_QUERY_RESULTS_KB,
+} from 'src/SqlLab/constants';
 import { queries, defaultQueryEditor } from '../fixtures';
 
 describe('reduxStateToLocalStorageHelper', () => {
@@ -45,6 +50,38 @@ describe('reduxStateToLocalStorageHelper', () => {
     expect(emptiedQuery[id].results).toEqual({});
   });
 
+  it('should empty query.results if query,.results size is greater than LOCALSTORAGE_MAX_QUERY_RESULTS_KB', () => {
+    const reasonableSizeQuery = {
+      ...queries[0],
+      startDttm: Date.now(),
+      results: { data: [{ a: 1 }] },
+    };
+    const largeQuery = {
+      ...queries[1],
+      startDttm: Date.now(),
+      results: {
+        data: [
+          {
+            jsonValue: `{"str":"${new Array(
+              (LOCALSTORAGE_MAX_QUERY_RESULTS_KB / BYTES_PER_CHAR) * KB_STORAGE,
+            )
+              .fill(0)
+              .join('')}"}`,
+          },
+        ],
+      },
+    };
+    expect(Object.keys(largeQuery.results)).toContain('data');
+    const emptiedQuery = emptyQueryResults({
+      [reasonableSizeQuery.id]: reasonableSizeQuery,
+      [largeQuery.id]: largeQuery,
+    });
+    expect(emptiedQuery[largeQuery.id].results).toEqual({});
+    expect(emptiedQuery[reasonableSizeQuery.id].results).toEqual(
+      reasonableSizeQuery.results,
+    );
+  });
+
   it('should only return selected keys for query editor', () => {
     const queryEditors = [defaultQueryEditor];
     expect(Object.keys(queryEditors[0])).toContain('schemaOptions');

superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js

@@ -16,7 +16,12 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../constants';
+import {
+  BYTES_PER_CHAR,
+  KB_STORAGE,
+  LOCALSTORAGE_MAX_QUERY_AGE_MS,
+  LOCALSTORAGE_MAX_QUERY_RESULTS_KB,
+} from '../constants';
 
 const PERSISTENT_QUERY_EDITOR_KEYS = new Set([
   'remoteId',
@@ -36,13 +41,21 @@ const PERSISTENT_QUERY_EDITOR_KEYS = new Set([
   'hideLeftBar',
 ]);
 
+function shouldEmptyQueryResults(query) {
+  const { startDttm, results } = query;
+  return (
+    Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ||
+    ((JSON.stringify(results)?.length || 0) * BYTES_PER_CHAR) / KB_STORAGE >
+      LOCALSTORAGE_MAX_QUERY_RESULTS_KB
+  );
+}
+
 export function emptyQueryResults(queries) {
   return Object.keys(queries).reduce((accu, key) => {
-    const { startDttm, results } = queries[key];
+    const { results } = queries[key];
     const query = {
       ...queries[key],
-      results:
-        Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS ? {} : results,
+      results: shouldEmptyQueryResults(queries[key]) ? {} : results,
     };
 
     const updatedQueries = {

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

@@ -186,7 +186,7 @@ test('Refresh should work', async () => {
   expect(fetchMock.calls(schemaApiRoute).length).toBe(0);
 
   const select = screen.getByRole('combobox', {
-    name: 'Select schema or type schema name',
+    name: 'Select schema or type to search schemas',
   });
 
   userEvent.click(select);
@@ -215,7 +215,7 @@ test('Should database select display options', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />, { useRedux: true });
   const select = screen.getByRole('combobox', {
-    name: 'Select database or type database name',
+    name: 'Select database or type to search databases',
   });
   expect(select).toBeInTheDocument();
   userEvent.click(select);
@@ -237,7 +237,7 @@ test('should show empty state if there are no options', async () => {
     { useRedux: true },
   );
   const select = screen.getByRole('combobox', {
-    name: 'Select database or type database name',
+    name: 'Select database or type to search databases',
   });
   userEvent.click(select);
   const emptystate = await screen.findByText('empty');
@@ -249,7 +249,7 @@ test('Should schema select display options', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />, { useRedux: true });
   const select = screen.getByRole('combobox', {
-    name: 'Select schema or type schema name',
+    name: 'Select schema or type to search schemas',
   });
   expect(select).toBeInTheDocument();
   userEvent.click(select);
@@ -265,7 +265,7 @@ test('Sends the correct db when changing the database', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />, { useRedux: true });
   const select = screen.getByRole('combobox', {
-    name: 'Select database or type database name',
+    name: 'Select database or type to search databases',
   });
   expect(select).toBeInTheDocument();
   userEvent.click(select);
@@ -285,7 +285,7 @@ test('Sends the correct schema when changing the schema', async () => {
   const props = createProps();
   render(<DatabaseSelector {...props} />, { useRedux: true });
   const select = screen.getByRole('combobox', {
-    name: 'Select schema or type schema name',
+    name: 'Select schema or type to search schemas',
   });
   expect(select).toBeInTheDocument();
   userEvent.click(select);

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

@@ -273,15 +273,15 @@ export default function DatabaseSelector({
   function renderDatabaseSelect() {
     return renderSelectRow(
       <AsyncSelect
-        ariaLabel={t('Select database or type database name')}
+        ariaLabel={t('Select database or type to search databases')}
         optionFilterProps={['database_name', 'value']}
         data-test="select-database"
         header={<FormLabel>{t('Database')}</FormLabel>}
         lazyLoading={false}
         notFoundContent={emptyState}
         onChange={changeDataBase}
         value={currentDb}
-        placeholder={t('Select database or type database name')}
+        placeholder={t('Select database or type to search databases')}
         disabled={!isDatabaseSelectEnabled || readOnly}
         options={loadDatabases}
       />,
@@ -298,14 +298,14 @@ export default function DatabaseSelector({
     );
     return renderSelectRow(
       <Select
-        ariaLabel={t('Select schema or type schema name')}
+        ariaLabel={t('Select schema or type to search schemas')}
         disabled={!currentDb || readOnly}
         header={<FormLabel>{t('Schema')}</FormLabel>}
         labelInValue
         loading={loadingSchemas}
         name="select-schema"
         notFoundContent={t('No compatible schema found')}
-        placeholder={t('Select schema or type schema name')}
+        placeholder={t('Select schema or type to search schemas')}
         onChange={item => changeSchema(item as SchemaOption)}
         options={schemaOptions}
         showSearch

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

@@ -158,11 +158,9 @@ export const EmptyStateBig = ({
   <EmptyStateContainer className={className}>
     {image && <ImageContainer image={image} size={EmptyStateSize.Big} />}
     <TextContainer
-      css={(theme: SupersetTheme) =>
-        css`
-          max-width: ${theme.gridUnit * 150}px;
-        `
-      }
+      css={(theme: SupersetTheme) => css`
+        max-width: ${theme.gridUnit * 150}px;
+      `}
     >
       <BigTitle>{title}</BigTitle>
       {description && <BigDescription>{description}</BigDescription>}
@@ -189,11 +187,9 @@ export const EmptyStateMedium = ({
   <EmptyStateContainer>
     {image && <ImageContainer image={image} size={EmptyStateSize.Medium} />}
     <TextContainer
-      css={(theme: SupersetTheme) =>
-        css`
-          max-width: ${theme.gridUnit * 100}px;
-        `
-      }
+      css={(theme: SupersetTheme) => css`
+        max-width: ${theme.gridUnit * 100}px;
+      `}
     >
       <Title>{title}</Title>
       {description && <Description>{description}</Description>}
@@ -218,11 +214,9 @@ export const EmptyStateSmall = ({
   <EmptyStateContainer>
     {image && <ImageContainer image={image} size={EmptyStateSize.Small} />}
     <TextContainer
-      css={(theme: SupersetTheme) =>
-        css`
-          max-width: ${theme.gridUnit * 75}px;
-        `
-      }
+      css={(theme: SupersetTheme) => css`
+        max-width: ${theme.gridUnit * 75}px;
+      `}
     >
       <Title>{title}</Title>
       {description && <SmallDescription>{description}</SmallDescription>}