fix(sqla): labels_expected contains mutated label (#14095)

(cherry picked from commit 667eb83e33)

superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx

@@ -61,7 +61,7 @@ describe('DatasourceEditor', () => {
   });
 
   it('renders Tabs', () => {
-    expect(wrapper.find(Tabs)).toExist();
+    expect(wrapper.find('#table-tabs')).toExist();
   });
 
   it('makes an async request', () =>

superset-frontend/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx

@@ -20,7 +20,8 @@ import React from 'react';
 import configureStore from 'redux-mock-store';
 import fetchMock from 'fetch-mock';
 import { shallow } from 'enzyme';
-import { render, screen, act } from '@testing-library/react';
+import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
 import { Provider } from 'react-redux';
 import '@testing-library/jest-dom/extend-expect';
 import thunk from 'redux-thunk';
@@ -56,6 +57,10 @@ describe('SqlEditorLeftBar', () => {
     });
   });
 
+  afterEach(() => {
+    wrapper.unmount();
+  });
+
   it('is valid', () => {
     expect(React.isValidElement(<SqlEditorLeftBar {...mockedProps} />)).toBe(
       true,
@@ -68,19 +73,14 @@ describe('SqlEditorLeftBar', () => {
 });
 
 describe('Left Panel Expansion', () => {
-  beforeEach(async () => {
-    await act(async () => {
-      render(
-        <ThemeProvider theme={supersetTheme}>
-          <Provider store={store}>
-            <SqlEditorLeftBar {...mockedProps} />
-          </Provider>
-        </ThemeProvider>,
-      );
-    });
-  });
-
-  it('table should be visible when expanded is true', async () => {
+  it('table should be visible when expanded is true', () => {
+    const { container } = render(
+      <ThemeProvider theme={supersetTheme}>
+        <Provider store={store}>
+          <SqlEditorLeftBar {...mockedProps} />
+        </Provider>
+      </ThemeProvider>,
+    );
     const dbSelect = screen.getByText(/select a database/i);
     const schemaSelect = screen.getByText(/select a schema \(0\)/i);
     const dropdown = screen.getByText(/Select table/i);
@@ -89,5 +89,28 @@ describe('Left Panel Expansion', () => {
     expect(schemaSelect).toBeInTheDocument();
     expect(dropdown).toBeInTheDocument();
     expect(abUser).toBeInTheDocument();
+    expect(
+      container.querySelector('.ant-collapse-content-active'),
+    ).toBeInTheDocument();
+  });
+
+  it('should toggle the table when the header is clicked', async () => {
+    const collapseMock = jest.fn();
+    render(
+      <ThemeProvider theme={supersetTheme}>
+        <Provider store={store}>
+          <SqlEditorLeftBar
+            actions={{ ...mockedActions, collapseTable: collapseMock }}
+            tables={[table]}
+            queryEditor={defaultQueryEditor}
+            database={databases}
+            height={0}
+          />
+        </Provider>
+      </ThemeProvider>,
+    );
+    const header = screen.getByText(/ab_user/);
+    userEvent.click(header);
+    expect(collapseMock).toHaveBeenCalled();
   });
 });

superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx

@@ -118,22 +118,6 @@ describe('TableElement', () => {
       'active',
     );
   });
-  it('calls the collapseTable action', () => {
-    const wrapper = mount(
-      <Provider store={store}>
-        <TableElement {...mockedProps} />
-      </Provider>,
-      {
-        wrappingComponent: ThemeProvider,
-        wrappingComponentProps: {
-          theme: supersetTheme,
-        },
-      },
-    );
-    expect(mockedActions.collapseTable.called).toBe(false);
-    wrapper.find('[data-test="collapse"]').hostNodes().simulate('click');
-    expect(mockedActions.collapseTable.called).toBe(true);
-  });
   it('removes the table', () => {
     const wrapper = mount(
       <Provider store={store}>

superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx

@@ -33,7 +33,7 @@ import {
   LOCALSTORAGE_MAX_QUERY_AGE_MS,
 } from '../../constants';
 
-const TAB_HEIGHT = 64;
+const TAB_HEIGHT = 90;
 
 /*
     editorQueries are queries executed by users passed from SqlEditor component
@@ -63,7 +63,6 @@ const StyledPane = styled.div`
     flex-direction: column;
   }
   .tab-content {
-    overflow: hidden;
     .alert {
       margin-top: ${({ theme }) => theme.gridUnit * 2}px;
     }

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

@@ -59,6 +59,7 @@ export default class SqlEditorLeftBar extends React.PureComponent {
     this.onDbChange = this.onDbChange.bind(this);
     this.getDbList = this.getDbList.bind(this);
     this.onTableChange = this.onTableChange.bind(this);
+    this.onToggleTable = this.onToggleTable.bind(this);
   }
 
   onSchemaChange(schema) {
@@ -91,6 +92,16 @@ export default class SqlEditorLeftBar extends React.PureComponent {
     this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
   }
 
+  onToggleTable(tables) {
+    this.props.tables.forEach(table => {
+      if (!tables.includes(table.id.toString()) && table.expanded) {
+        this.props.actions.collapseTable(table);
+      } else if (tables.includes(table.id.toString()) && !table.expanded) {
+        this.props.actions.expandTable(table);
+      }
+    });
+  }
+
   getDbList(dbs) {
     this.props.actions.setDatabases(dbs);
   }
@@ -172,13 +183,13 @@ export default class SqlEditorLeftBar extends React.PureComponent {
               `}
               expandIconPosition="right"
               ghost
+              onChange={this.onToggleTable}
             >
               {this.props.tables.map(table => (
                 <TableElement
                   table={table}
                   key={table.id}
                   actions={this.props.actions}
-                  onClick={this.toggleTable}
                 />
               ))}
             </Collapse>

superset-frontend/src/SqlLab/components/TableElement.jsx

@@ -79,15 +79,6 @@ class TableElement extends React.PureComponent {
     this.props.actions.addQueryEditor(qe);
   }
 
-  toggleTable(e) {
-    e.preventDefault();
-    if (this.props.table.expanded) {
-      this.props.actions.collapseTable(this.props.table);
-    } else {
-      this.props.actions.expandTable(this.props.table);
-    }
-  }
-
   removeTable() {
     this.props.actions.removeDataPreview(this.props.table);
     this.props.actions.removeTable(this.props.table);
@@ -214,13 +205,7 @@ class TableElement extends React.PureComponent {
           title={table.name}
           trigger={['hover']}
         >
-          <StyledSpan
-            data-test="collapse"
-            className="table-name"
-            onClick={e => {
-              this.toggleTable(e);
-            }}
-          >
+          <StyledSpan data-test="collapse" className="table-name">
             <strong>{table.name}</strong>
           </StyledSpan>
         </Tooltip>

superset-frontend/src/SqlLab/main.less

@@ -56,7 +56,7 @@ body {
   position: relative;
   background-color: @lightest;
   overflow-x: auto;
-  overflow-y: hidden;
+  overflow-y: auto;
 
   > .ant-tabs-tabpane {
     position: absolute;
@@ -300,6 +300,7 @@ div.Workspace {
   .schemaPane-enter-done,
   .schemaPane-exit {
     transform: translateX(0);
+    z-index: 7;
   }
 
   .schemaPane-exit-active {

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

@@ -245,7 +245,6 @@ export default function DatabaseSelector({
         placeholder={t('Select a database')}
         autoSelect
         isDisabled={!isDatabaseSelectEnabled || readOnly}
-        menuPosition="fixed"
       />,
       null,
     );

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

@@ -30,6 +30,10 @@ const ProgressBar = styled(({ striped, ...props }: ProgressBarProps) => (
   <AntdProgress {...props} />
 ))`
   line-height: 0;
+  position: static;
+  .ant-progress-inner {
+    position: static;
+  }
   .ant-progress-outer {
     ${({ percent }) => !percent && `display: none;`}
   }
@@ -37,6 +41,7 @@ const ProgressBar = styled(({ striped, ...props }: ProgressBarProps) => (
     font-size: ${({ theme }) => theme.typography.sizes.s}px;
   }
   .ant-progress-bg {
+    position: static;
     ${({ striped }) =>
       striped &&
       `

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

@@ -326,7 +326,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
           optionRenderer={renderTableOption}
           valueRenderer={renderTableOption}
           isDisabled={readOnly}
-          menuPosition="fixed"
         />
       );
     } else if (formMode) {

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

@@ -112,6 +112,10 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
   const filters = useFilters();
   const filterValues = Object.values<Filter>(filters);
 
+  const nativeFiltersEnabled = isFeatureEnabled(
+    FeatureFlag.DASHBOARD_NATIVE_FILTERS,
+  );
+
   const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState(true);
 
   const toggleDashboardFiltersOpen = (visible?: boolean) => {
@@ -150,7 +154,11 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
     (topLevelTabs ? TABS_HEIGHT : 0);
 
   useEffect(() => {
-    if (filterValues.length === 0 && dashboardFiltersOpen) {
+    if (
+      filterValues.length === 0 &&
+      dashboardFiltersOpen &&
+      nativeFiltersEnabled
+    ) {
       toggleDashboardFiltersOpen(false);
     }
   }, [filterValues.length]);
@@ -215,7 +223,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
         className="dashboard-content"
         dashboardFiltersOpen={dashboardFiltersOpen}
       >
-        {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && !editMode && (
+        {nativeFiltersEnabled && !editMode && (
           <StickyVerticalBar
             filtersOpen={dashboardFiltersOpen}
             topOffset={barTopOffset}

superset-frontend/src/datasource/DatasourceEditor.jsx

@@ -79,6 +79,13 @@ const FlexRowContainer = styled.div`
   }
 `;
 
+const StyledTableTabs = styled(Tabs)`
+  overflow: visible;
+  .ant-tabs-content-holder {
+    overflow: visible;
+  }
+`;
+
 const EditLockContainer = styled.div`
   font-size: ${supersetTheme.typography.sizes.s}px;
   display: flex;
@@ -995,7 +1002,7 @@ class DatasourceEditor extends React.PureComponent {
             </>
           }
         />
-        <Tabs
+        <StyledTableTabs
           fullWidth={false}
           id="table-tabs"
           data-test="edit-dataset-tabs"
@@ -1086,7 +1093,7 @@ class DatasourceEditor extends React.PureComponent {
               </Col>
             </div>
           </Tabs.TabPane>
-        </Tabs>
+        </StyledTableTabs>
       </DatasourceContainer>
     );
   }

superset-frontend/src/datasource/DatasourceModal.tsx

@@ -49,6 +49,10 @@ const StyledDatasourceModal = styled(Modal)`
   .modal-footer {
     flex: 0 1 auto;
   }
+
+  .ant-modal-body {
+    overflow: visible;
+  }
 `;
 
 interface DatasourceModalProps {

superset-frontend/src/explore/components/controls/VizTypeControl.jsx

@@ -107,6 +107,11 @@ function VizSupportValidation({ vizType }) {
   );
 }
 
+const nativeFilterGate = behaviors =>
+  !behaviors.includes(Behavior.NATIVE_FILTER) ||
+  (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
+    behaviors.includes(Behavior.CROSS_FILTER));
+
 const VizTypeControl = props => {
   const [showModal, setShowModal] = useState(false);
   const [filter, setFilter] = useState('');
@@ -168,11 +173,7 @@ const VizTypeControl = props => {
   const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
     .filter(type => {
       const behaviors = registry.get(type)?.behaviors || [];
-      return (
-        (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
-          behaviors.includes(Behavior.CROSS_FILTER)) ||
-        !behaviors.length
-      );
+      return nativeFilterGate(behaviors);
     })
     .map(type => ({
       key: type,
@@ -183,11 +184,7 @@ const VizTypeControl = props => {
         .entries()
         .filter(entry => {
           const behaviors = entry.value?.behaviors || [];
-          return (
-            (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
-              behaviors.includes(Behavior.CROSS_FILTER)) ||
-            !behaviors.length
-          );
+          return nativeFilterGate(behaviors);
         })
         .filter(({ key }) => !typesWithDefaultOrder.has(key)),
     )

superset-frontend/src/views/CRUD/alert/ExecutionLog.tsx

@@ -100,7 +100,7 @@ function ExecutionLog({ addDangerToast, isReportEnabled }: ExecutionLogProps) {
           row: {
             original: { uuid: executionId },
           },
-        }: any) => executionId.slice(0, 6),
+        }: any) => (executionId ? executionId.slice(0, 6) : 'none'),
         accessor: 'uuid',
         Header: t('Execution ID'),
         size: 'xs',

superset/charts/schemas.py

@@ -21,7 +21,9 @@ from marshmallow import EXCLUDE, fields, post_load, Schema, validate
 from marshmallow.validate import Length, Range
 from marshmallow_enum import EnumField
 
+from superset import app
 from superset.common.query_context import QueryContext
+from superset.db_engine_specs.base import builtin_time_grains
 from superset.utils import schema as utils
 from superset.utils.core import (
     AnnotationType,
@@ -33,6 +35,8 @@ from superset.utils.core import (
     TimeRangeEndpoint,
 )
 
+config = app.config
+
 #
 # RISON/JSON schemas for query parameters
 #
@@ -126,26 +130,6 @@ openapi_spec_methods_override = {
 }
 
 
-TIME_GRAINS = (
-    "PT1S",
-    "PT1M",
-    "PT5M",
-    "PT10M",
-    "PT15M",
-    "PT0.5H",
-    "PT1H",
-    "P1D",
-    "P1W",
-    "P1M",
-    "P0.25Y",
-    "P1Y",
-    "1969-12-28T00:00:00Z/P1W",  # Week starting Sunday
-    "1969-12-29T00:00:00Z/P1W",  # Week starting Monday
-    "P1W/1970-01-03T00:00:00Z",  # Week ending Saturday
-    "P1W/1970-01-04T00:00:00Z",  # Week ending Sunday
-)
-
-
 class ChartEntityResponseSchema(Schema):
     """
     Schema for a chart object
@@ -498,7 +482,13 @@ class ChartDataProphetOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
         description="Time grain used to specify time period increments in prediction. "
         "Supports [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Durations) "
         "durations.",
-        validate=validate.OneOf(choices=TIME_GRAINS),
+        validate=validate.OneOf(
+            choices=[
+                i
+                for i in {**builtin_time_grains, **config["TIME_GRAIN_ADDONS"]}.keys()
+                if i
+            ]
+        ),
         example="P1D",
         required=True,
     )
@@ -796,7 +786,13 @@ class ChartDataExtrasSchema(Schema):
         description="To what level of granularity should the temporal column be "
         "aggregated. Supports "
         "[ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Durations) durations.",
-        validate=validate.OneOf(choices=TIME_GRAINS),
+        validate=validate.OneOf(
+            choices=[
+                i
+                for i in {**builtin_time_grains, **config["TIME_GRAIN_ADDONS"]}.keys()
+                if i
+            ]
+        ),
         example="P1D",
         allow_none=True,
     )

superset/config.py

@@ -359,6 +359,13 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
     "OMNIBAR": False,
     "DASHBOARD_RBAC": False,
     "ENABLE_EXPLORE_DRAG_AND_DROP": False,
+    # Enabling ALERTS_ATTACH_REPORTS, the system sends email and slack message
+    # with screenshot and link
+    # Disables ALERTS_ATTACH_REPORTS, the system DOES NOT generate screenshot
+    # for report with type 'alert' and sends email and slack message with only link;
+    # for report with type 'report' still send with email and slack message with
+    # screenshot and link
+    "ALERTS_ATTACH_REPORTS": True,
 }
 
 # Set the default view to card/grid view if thumbnail support is enabled.

superset/connectors/sqla/models.py

@@ -521,6 +521,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
         if db_engine_spec.allows_alias_in_select:
             label = db_engine_spec.make_label_compatible(label_expected)
             sqla_col = sqla_col.label(label)
+        sqla_col.key = label_expected
         return sqla_col
 
     def __repr__(self) -> str:
@@ -1094,7 +1095,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
             )
 
         select_exprs += metrics_exprs
-        labels_expected = [c.name for c in select_exprs]
+        labels_expected = [c.key for c in select_exprs]
         select_exprs = db_engine_spec.make_select_compatible(
             groupby_exprs_with_timestamp.values(), select_exprs
         )

superset/dashboards/commands/importers/v1/utils.py

@@ -115,7 +115,7 @@ def import_dashboard(
     # TODO (betodealmeida): move this logic to import_from_dict
     config = config.copy()
     for key, new_name in JSON_KEYS.items():
-        if config.get(key):
+        if config.get(key) is not None:
             value = config.pop(key)
             try:
                 config[new_name] = json.dumps(value)

superset/datasets/api.py

@@ -314,6 +314,8 @@ class DatasetRestApi(BaseSupersetModelRestApi):
             changed_model = UpdateDatasetCommand(
                 g.user, pk, item, override_columns
             ).run()
+            if override_columns:
+                RefreshDatasetCommand(g.user, pk).run()
             response = self.response(200, id=changed_model.id, result=item)
         except DatasetNotFoundError:
             response = self.response_404()

superset/datasets/commands/importers/v1/utils.py

@@ -92,7 +92,7 @@ def import_dataset(
     # TODO (betodealmeida): move this logic to import_from_dict
     config = config.copy()
     for key in JSON_KEYS:
-        if config.get(key):
+        if config.get(key) is not None:
             try:
                 config[key] = json.dumps(config[key])
             except TypeError:

superset/reports/commands/execute.py

@@ -26,6 +26,7 @@ from sqlalchemy.orm import Session
 from superset import app
 from superset.commands.base import BaseCommand
 from superset.commands.exceptions import CommandException
+from superset.extensions import feature_flag_manager
 from superset.models.reports import (
     ReportExecutionLog,
     ReportSchedule,
@@ -52,7 +53,7 @@ from superset.reports.dao import (
     ReportScheduleDAO,
 )
 from superset.reports.notifications import create_notification
-from superset.reports.notifications.base import NotificationContent, ScreenshotData
+from superset.reports.notifications.base import NotificationContent
 from superset.reports.notifications.exceptions import NotificationError
 from superset.utils.celery import session_scope
 from superset.utils.screenshots import (
@@ -153,7 +154,7 @@ class BaseReportState:
             raise ReportScheduleSelleniumUserNotFoundError()
         return user
 
-    def _get_screenshot(self) -> ScreenshotData:
+    def _get_screenshot(self) -> bytes:
         """
         Get a chart or dashboard screenshot
 
@@ -176,7 +177,6 @@ class BaseReportState:
                 window_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
                 thumb_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
             )
-        image_url = self._get_url(user_friendly=True)
         user = self._get_screenshot_user()
         try:
             image_data = screenshot.get_screenshot(user=user)
@@ -188,7 +188,7 @@ class BaseReportState:
             )
         if not image_data:
             raise ReportScheduleScreenshotFailedError()
-        return ScreenshotData(url=image_url, image=image_data)
+        return image_data
 
     def _get_notification_content(self) -> NotificationContent:
         """
@@ -196,7 +196,19 @@ class BaseReportState:
 
         :raises: ReportScheduleScreenshotFailedError
         """
-        screenshot_data = self._get_screenshot()
+        screenshot_data = None
+        url = self._get_url(user_friendly=True)
+        if (
+            feature_flag_manager.is_feature_enabled("ALERTS_ATTACH_REPORTS")
+            or self._report_schedule.type == ReportScheduleType.REPORT
+        ):
+            screenshot_data = self._get_screenshot()
+            if not screenshot_data:
+                return NotificationContent(
+                    name=self._report_schedule.name,
+                    text="Unexpected missing screenshot",
+                )
+
         if self._report_schedule.chart:
             name = (
                 f"{self._report_schedule.name}: "
@@ -207,7 +219,7 @@ class BaseReportState:
                 f"{self._report_schedule.name}: "
                 f"{self._report_schedule.dashboard.dashboard_title}"
             )
-        return NotificationContent(name=name, screenshot=screenshot_data)
+        return NotificationContent(name=name, url=url, screenshot=screenshot_data)
 
     def _send(self, notification_content: NotificationContent) -> None:
         """

superset/reports/notifications/base.py

@@ -21,16 +21,11 @@ from typing import Any, List, Optional, Type
 from superset.models.reports import ReportRecipients, ReportRecipientType
 
 
-@dataclass
-class ScreenshotData:
-    url: str  # url to chart/dashboard for this screenshot
-    image: bytes  # bytes for the screenshot
-
-
 @dataclass
 class NotificationContent:
     name: str
-    screenshot: Optional[ScreenshotData] = None
+    url: Optional[str] = None  # url to chart/dashboard for this screenshot
+    screenshot: Optional[bytes] = None  # bytes for the screenshot
     text: Optional[str] = None
 
 

superset/reports/notifications/email.py

@@ -63,21 +63,21 @@ class EmailNotification(BaseNotification):  # pylint: disable=too-few-public-met
             return EmailContent(body=self._error_template(self._content.text))
         # Get the domain from the 'From' address ..
         # and make a message id without the < > in the end
+        image = None
+        domain = self._get_smtp_domain()
+        msgid = make_msgid(domain)[1:-1]
+        body = __(
+            """
+            <b><a href="%(url)s">Explore in Superset</a></b><p></p>
+            <img src="cid:%(msgid)s">
+            """,
+            url=self._content.url,
+            msgid=msgid,
+        )
         if self._content.screenshot:
-            domain = self._get_smtp_domain()
-            msgid = make_msgid(domain)[1:-1]
+            image = {msgid: self._content.screenshot}
 
-            image = {msgid: self._content.screenshot.image}
-            body = __(
-                """
-                <b><a href="%(url)s">Explore in Superset</a></b><p></p>
-                <img src="cid:%(msgid)s">
-                """,
-                url=self._content.screenshot.url,
-                msgid=msgid,
-            )
-            return EmailContent(body=body, images=image)
-        return EmailContent(body=self._error_template("Unexpected missing screenshot"))
+        return EmailContent(body=body, images=image)
 
     def _get_subject(self) -> str:
         return __(

superset/reports/notifications/slack.py

@@ -57,20 +57,18 @@ class SlackNotification(BaseNotification):  # pylint: disable=too-few-public-met
     def _get_body(self) -> str:
         if self._content.text:
             return self._error_template(self._content.name, self._content.text)
-        if self._content.screenshot:
-            return __(
-                """
-                *%(name)s*\n
-                <%(url)s|Explore in Superset>
-                """,
-                name=self._content.name,
-                url=self._content.screenshot.url,
-            )
-        return self._error_template(self._content.name, "Unexpected missing screenshot")
+        return __(
+            """
+            *%(name)s*\n
+            <%(url)s|Explore in Superset>
+            """,
+            name=self._content.name,
+            url=self._content.url,
+        )
 
     def _get_inline_screenshot(self) -> Optional[Union[str, IOBase, bytes]]:
         if self._content.screenshot:
-            return self._content.screenshot.image
+            return self._content.screenshot
         return None
 
     @retry(SlackApiError, delay=10, backoff=2, tries=5)

tests/datasets/api_tests.py

@@ -598,12 +598,7 @@ class TestDatasetApi(SupersetTestCase):
         rv = self.put_assert_metric(uri, dataset_data, "put")
         assert rv.status_code == 200
 
-        columns = (
-            db.session.query(TableColumn)
-            .filter_by(table_id=dataset.id)
-            .order_by("column_name")
-            .all()
-        )
+        columns = db.session.query(TableColumn).filter_by(table_id=dataset.id).all()
 
         assert columns[0].column_name == dataset_data["columns"][0]["column_name"]
         assert columns[0].description == dataset_data["columns"][0]["description"]

tests/datasets/commands_tests.py

@@ -312,7 +312,7 @@ class TestImportDatasetsCommand(SupersetTestCase):
         assert dataset.schema == ""
         assert dataset.sql == ""
         assert dataset.params is None
-        assert dataset.template_params is None
+        assert dataset.template_params == "{}"
         assert dataset.filter_select_enabled
         assert dataset.fetch_values_predicate is None
         assert dataset.extra is None

tests/fixtures/importexport.py

@@ -368,7 +368,7 @@ dataset_config: Dict[str, Any] = {
     "schema": "",
     "sql": "",
     "params": None,
-    "template_params": None,
+    "template_params": {},
     "filter_select_enabled": True,
     "fetch_values_predicate": None,
     "extra": None,

tests/reports/commands_tests.py

@@ -750,6 +750,10 @@ def test_email_dashboard_report_fails(
 )
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    ALERTS_ATTACH_REPORTS=True,
+)
 def test_slack_chart_alert(screenshot_mock, email_mock, create_alert_email_chart):
     """
     ExecuteReport Command: Test chart slack alert
@@ -773,6 +777,34 @@ def test_slack_chart_alert(screenshot_mock, email_mock, create_alert_email_chart
         assert_log(ReportState.SUCCESS)
 
 
+@pytest.mark.usefixtures(
+    "load_birth_names_dashboard_with_slices", "create_alert_email_chart"
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    ALERTS_ATTACH_REPORTS=False,
+)
+def test_slack_chart_alert_no_attachment(email_mock, create_alert_email_chart):
+    """
+    ExecuteReport Command: Test chart slack alert
+    """
+    # setup screenshot mock
+
+    with freeze_time("2020-01-01T00:00:00Z"):
+        AsyncExecuteReportScheduleCommand(
+            test_id, create_alert_email_chart.id, datetime.utcnow()
+        ).run()
+
+        notification_targets = get_target_from_report_schedule(create_alert_email_chart)
+        # Assert the email smtp address
+        assert email_mock.call_args[0][0] == notification_targets[0]
+        # Assert the there is no attached image
+        assert email_mock.call_args[1]["images"] is None
+        # Assert logs are correct
+        assert_log(ReportState.SUCCESS)
+
+
 @pytest.mark.usefixtures(
     "load_birth_names_dashboard_with_slices", "create_report_slack_chart"
 )
@@ -859,6 +891,10 @@ def test_soft_timeout_alert(email_mock, create_alert_email_chart):
 )
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    ALERTS_ATTACH_REPORTS=True,
+)
 def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email_chart):
     """
     ExecuteReport Command: Test soft timeout on screenshot
@@ -882,11 +918,11 @@ def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email
 
 
 @pytest.mark.usefixtures(
-    "load_birth_names_dashboard_with_slices", "create_alert_email_chart"
+    "load_birth_names_dashboard_with_slices", "create_report_email_chart"
 )
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
-def test_fail_screenshot(screenshot_mock, email_mock, create_alert_email_chart):
+def test_fail_screenshot(screenshot_mock, email_mock, create_report_email_chart):
     """
     ExecuteReport Command: Test soft timeout on screenshot
     """
@@ -896,10 +932,10 @@ def test_fail_screenshot(screenshot_mock, email_mock, create_alert_email_chart):
     screenshot_mock.side_effect = Exception("Unexpected error")
     with pytest.raises(ReportScheduleScreenshotFailedError):
         AsyncExecuteReportScheduleCommand(
-            test_id, create_alert_email_chart.id, datetime.utcnow()
+            test_id, create_report_email_chart.id, datetime.utcnow()
         ).run()
 
-    notification_targets = get_target_from_report_schedule(create_alert_email_chart)
+    notification_targets = get_target_from_report_schedule(create_report_email_chart)
     # Assert the email smtp address, asserts a notification was sent with the error
     assert email_mock.call_args[0][0] == notification_targets[0]
 
@@ -908,6 +944,32 @@ def test_fail_screenshot(screenshot_mock, email_mock, create_alert_email_chart):
     )
 
 
+@pytest.mark.usefixtures(
+    "load_birth_names_dashboard_with_slices", "create_alert_email_chart"
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    ALERTS_ATTACH_REPORTS=False,
+)
+def test_email_disable_screenshot(email_mock, create_alert_email_chart):
+    """
+    ExecuteReport Command: Test soft timeout on screenshot
+    """
+
+    AsyncExecuteReportScheduleCommand(
+        test_id, create_alert_email_chart.id, datetime.utcnow()
+    ).run()
+
+    notification_targets = get_target_from_report_schedule(create_alert_email_chart)
+    # Assert the email smtp address, asserts a notification was sent with the error
+    assert email_mock.call_args[0][0] == notification_targets[0]
+    # Assert the there is no attached image
+    assert email_mock.call_args[1]["images"] is None
+
+    assert_log(ReportState.SUCCESS)
+
+
 @pytest.mark.usefixtures("create_invalid_sql_alert_email_chart")
 @patch("superset.reports.notifications.email.send_email_smtp")
 def test_invalid_sql_alert(email_mock, create_invalid_sql_alert_email_chart):

tests/sqla_models_tests.py

@@ -22,6 +22,7 @@ import pytest
 
 from superset import db
 from superset.connectors.sqla.models import SqlaTable, TableColumn
+from superset.db_engine_specs.bigquery import BigQueryEngineSpec
 from superset.db_engine_specs.druid import DruidEngineSpec
 from superset.exceptions import QueryObjectValidationError
 from superset.models.core import Database
@@ -305,3 +306,36 @@ class TestDatabaseModel(SupersetTestCase):
         assert cols["mycase"].expression == ""
         assert VIRTUAL_TABLE_STRING_TYPES[backend].match(cols["mycase"].type)
         assert cols["expr"].expression == "case when 1 then 1 else 0 end"
+
+    @patch("superset.models.core.Database.db_engine_spec", BigQueryEngineSpec)
+    def test_labels_expected_on_mutated_query(self):
+        query_obj = {
+            "granularity": None,
+            "from_dttm": None,
+            "to_dttm": None,
+            "groupby": ["user"],
+            "metrics": [
+                {
+                    "expressionType": "SIMPLE",
+                    "column": {"column_name": "user"},
+                    "aggregate": "COUNT_DISTINCT",
+                    "label": "COUNT_DISTINCT(user)",
+                }
+            ],
+            "is_timeseries": False,
+            "filter": [],
+            "extras": {},
+        }
+
+        database = Database(database_name="testdb", sqlalchemy_uri="sqlite://")
+        table = SqlaTable(table_name="bq_table", database=database)
+        db.session.add(database)
+        db.session.add(table)
+        db.session.commit()
+        sqlaq = table.get_sqla_query(**query_obj)
+        assert sqlaq.labels_expected == ["user", "COUNT_DISTINCT(user)"]
+        sql = table.database.compile_sqla_query(sqlaq.sqla_query)
+        assert "COUNT_DISTINCT_user__00db1" in sql
+        db.session.delete(table)
+        db.session.delete(database)
+        db.session.commit()