feat(report): allow capturing dashboard reports in specific state

superset/dashboards/permalink/commands/create.py

@@ -31,8 +31,10 @@
 
 class CreateDashboardPermalinkCommand(BaseDashboardPermalinkCommand):
     """
-    Get or create a permalink key for the given dashboard in certain state.
-    Will reuse the key for the same user and dashboard state.
+    Get or create a permalink key for the dashboard.
+
+    The same dashboard_id and state for the same user will return the
+    same permalink.
     """
 
     def __init__(

superset/migrations/versions/2022-05-03_19-39_cbe71abde154_fix_report_schedule_and_log.py

@@ -31,7 +31,7 @@
 from sqlalchemy.ext.declarative import declarative_base
 
 from superset import db
-from superset.models.reports import ReportState
+from superset.reports.models import ReportState
 
 Base = declarative_base()
 

superset/migrations/versions/2022-07-11_11-26_ffa79af61a56_rename_report_schedule_extra_to_extra_.py

@@ -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.
+"""rename report_schedule.extra to extra_json
+
+So we can reuse the ExtraJSONMixin
+
+Revision ID: ffa79af61a56
+Revises: 409c7b420ab0
+Create Date: 2022-07-11 11:26:00.010714
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "ffa79af61a56"
+down_revision = "409c7b420ab0"
+
+from alembic import op
+from sqlalchemy.types import Text
+
+
+def upgrade():
+    op.alter_column(
+        "report_schedule",
+        "extra",
+        new_column_name="extra_json",
+        # existing info is required for MySQL
+        existing_type=Text,
+        existing_nullable=True,
+    )
+
+
+def downgrade():
+    op.alter_column(
+        "report_schedule",
+        "extra_json",
+        new_column_name="extra",
+        existing_type=Text,
+        existing_nullable=True,
+    )

superset/models/dashboard.py

@@ -438,13 +438,15 @@ def export_dashboards(  # pylint: disable=too-many-locals
         )
 
     @classmethod
-    def get(cls, id_or_slug: str) -> Dashboard:
+    def get(cls, id_or_slug: Union[str, int]) -> Dashboard:
         session = db.session()
         qry = session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
         return qry.one_or_none()
 
 
-def id_or_slug_filter(id_or_slug: str) -> BinaryExpression:
+def id_or_slug_filter(id_or_slug: Union[int, str]) -> BinaryExpression:
+    if isinstance(id_or_slug, int):
+        return Dashboard.id == id_or_slug
     if id_or_slug.isdigit():
         return Dashboard.id == int(id_or_slug)
     return Dashboard.slug == id_or_slug

superset/models/helpers.py

@@ -55,7 +55,7 @@
 from jinja2.exceptions import TemplateError
 from sqlalchemy import and_, Column, or_, UniqueConstraint
 from sqlalchemy.ext.declarative import declared_attr
-from sqlalchemy.orm import Mapper, Session
+from sqlalchemy.orm import Mapper, Session, validates
 from sqlalchemy.orm.exc import MultipleResultsFound
 from sqlalchemy.sql.elements import ColumnElement, literal_column, TextClause
 from sqlalchemy.sql.expression import Label, Select, TextAsFrom
@@ -567,21 +567,32 @@ class ExtraJSONMixin:
     @property
     def extra(self) -> Dict[str, Any]:
         try:
-            return json.loads(self.extra_json) if self.extra_json else {}
+            return json.loads(self.extra_json or "{}") or {}
         except (TypeError, JSONDecodeError) as exc:
             logger.error(
                 "Unable to load an extra json: %r. Leaving empty.", exc, exc_info=True
             )
             return {}
 
-    def set_extra_json(self, extras: Dict[str, Any]) -> None:
+    @extra.setter
+    def extra(self, extras: Dict[str, Any]) -> None:
         self.extra_json = json.dumps(extras)
 
     def set_extra_json_key(self, key: str, value: Any) -> None:
         extra = self.extra
         extra[key] = value
         self.extra_json = json.dumps(extra)
 
+    @validates("extra_json")
+    def ensure_extra_json_is_not_none(  # pylint: disable=no-self-use
+        self,
+        _: str,
+        value: Optional[Dict[str, Any]],
+    ) -> Any:
+        if value is None:
+            return "{}"
+        return value
+
 
 class CertificationMixin:
     """Mixin to add extra certification fields"""

superset/reports/api.py

@@ -30,7 +30,6 @@
 from superset.dashboards.filters import DashboardAccessFilter
 from superset.databases.filters import DatabaseFilter
 from superset.extensions import event_logger
-from superset.models.reports import ReportSchedule
 from superset.reports.commands.bulk_delete import BulkDeleteReportScheduleCommand
 from superset.reports.commands.create import CreateReportScheduleCommand
 from superset.reports.commands.delete import DeleteReportScheduleCommand
@@ -45,6 +44,7 @@
 )
 from superset.reports.commands.update import UpdateReportScheduleCommand
 from superset.reports.filters import ReportScheduleAllTextFilter
+from superset.reports.models import ReportSchedule
 from superset.reports.schemas import (
     get_delete_ids_schema,
     openapi_spec_methods_override,
@@ -94,6 +94,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "database.database_name",
         "database.id",
         "description",
+        "extra",
         "force_screenshot",
         "grace_period",
         "last_eval_dttm",
@@ -135,6 +136,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "crontab_humanized",
         "dashboard_id",
         "description",
+        "extra",
         "id",
         "last_eval_dttm",
         "last_state",
@@ -156,6 +158,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "dashboard",
         "database",
         "description",
+        "extra",
         "force_screenshot",
         "grace_period",
         "log_retention",

superset/reports/commands/alert.py

@@ -27,7 +27,6 @@
 
 from superset import app, jinja_context, security_manager
 from superset.commands.base import BaseCommand
-from superset.models.reports import ReportSchedule, ReportScheduleValidatorType
 from superset.reports.commands.exceptions import (
     AlertQueryError,
     AlertQueryInvalidTypeError,
@@ -36,6 +35,7 @@
     AlertQueryTimeout,
     AlertValidatorConfigError,
 )
+from superset.reports.models import ReportSchedule, ReportScheduleValidatorType
 from superset.utils.core import override_user
 from superset.utils.retries import retry_call
 

superset/reports/commands/base.py

@@ -22,14 +22,15 @@
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand
 from superset.dashboards.dao import DashboardDAO
-from superset.models.reports import ReportCreationMethod
 from superset.reports.commands.exceptions import (
     ChartNotFoundValidationError,
     ChartNotSavedValidationError,
     DashboardNotFoundValidationError,
     DashboardNotSavedValidationError,
-    ReportScheduleChartOrDashboardValidationError,
+    ReportScheduleEitherChartOrDashboardError,
+    ReportScheduleOnlyChartOrDashboardError,
 )
+from superset.reports.models import ReportCreationMethod
 
 logger = logging.getLogger(__name__)
 
@@ -62,7 +63,8 @@ def validate_chart_dashboard(
             return
 
         if chart_id and dashboard_id:
-            exceptions.append(ReportScheduleChartOrDashboardValidationError())
+            exceptions.append(ReportScheduleOnlyChartOrDashboardError())
+
         if chart_id:
             chart = ChartDAO.find_by_id(chart_id)
             if not chart:
@@ -74,4 +76,4 @@ def validate_chart_dashboard(
                 exceptions.append(DashboardNotFoundValidationError())
             self._properties["dashboard"] = dashboard
         elif not update:
-            exceptions.append(ReportScheduleChartOrDashboardValidationError())
+            exceptions.append(ReportScheduleEitherChartOrDashboardError())

superset/reports/commands/bulk_delete.py

@@ -21,13 +21,13 @@
 from superset.commands.base import BaseCommand
 from superset.dao.exceptions import DAODeleteFailedError
 from superset.exceptions import SupersetSecurityException
-from superset.models.reports import ReportSchedule
 from superset.reports.commands.exceptions import (
     ReportScheduleBulkDeleteFailedError,
     ReportScheduleForbiddenError,
     ReportScheduleNotFoundError,
 )
 from superset.reports.dao import ReportScheduleDAO
+from superset.reports.models import ReportSchedule
 
 logger = logging.getLogger(__name__)
 

superset/reports/commands/create.py

@@ -18,13 +18,12 @@
 import logging
 from typing import Any, Dict, List, Optional
 
-from flask_appbuilder.models.sqla import Model
+from flask_babel import gettext as _
 from marshmallow import ValidationError
 
 from superset.commands.base import CreateMixin
 from superset.dao.exceptions import DAOCreateFailedError
 from superset.databases.dao import DatabaseDAO
-from superset.models.reports import ReportCreationMethod, ReportScheduleType
 from superset.reports.commands.base import BaseReportScheduleCommand
 from superset.reports.commands.exceptions import (
     DatabaseNotFoundValidationError,
@@ -36,6 +35,12 @@
     ReportScheduleRequiredTypeValidationError,
 )
 from superset.reports.dao import ReportScheduleDAO
+from superset.reports.models import (
+    ReportCreationMethod,
+    ReportSchedule,
+    ReportScheduleType,
+)
+from superset.reports.types import ReportScheduleExtra
 
 logger = logging.getLogger(__name__)
 
@@ -44,7 +49,7 @@ class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand):
     def __init__(self, data: Dict[str, Any]):
         self._properties = data.copy()
 
-    def run(self) -> Model:
+    def run(self) -> ReportSchedule:
         self.validate()
         try:
             report_schedule = ReportScheduleDAO.create(self._properties)
@@ -117,20 +122,26 @@ def validate(self) -> None:
             raise exception
 
     def _validate_report_extra(self, exceptions: List[ValidationError]) -> None:
-        extra = self._properties.get("extra")
+        extra: Optional[ReportScheduleExtra] = self._properties.get("extra")
         dashboard = self._properties.get("dashboard")
 
         if extra is None or dashboard is None:
             return
 
-        dashboard_tab_ids = extra.get("dashboard_tab_ids")
-        if dashboard_tab_ids is None:
+        dashboard_state = extra.get("dashboard")
+        if not dashboard_state:
             return
-        position_data = json.loads(dashboard.position_json)
-        invalid_tab_ids = [
-            tab_id for tab_id in dashboard_tab_ids if tab_id not in position_data
-        ]
+
+        position_data = json.loads(dashboard.position_json or "{}")
+        active_tabs = dashboard_state.get("activeTabs") or []
+        anchor = dashboard_state.get("anchor")
+        invalid_tab_ids = set(active_tabs) - set(position_data.keys())
+        if anchor and anchor not in position_data:
+            invalid_tab_ids.add(anchor)
         if invalid_tab_ids:
             exceptions.append(
-                ValidationError(f"Invalid tab IDs selected: {invalid_tab_ids}", "extra")
+                ValidationError(
+                    _("Invalid tab ids: %s(tab_ids)", tab_ids=str(invalid_tab_ids)),
+                    "extra",
+                )
             )

superset/reports/commands/delete.py

@@ -23,13 +23,13 @@
 from superset.commands.base import BaseCommand
 from superset.dao.exceptions import DAODeleteFailedError
 from superset.exceptions import SupersetSecurityException
-from superset.models.reports import ReportSchedule
 from superset.reports.commands.exceptions import (
     ReportScheduleDeleteFailedError,
     ReportScheduleForbiddenError,
     ReportScheduleNotFoundError,
 )
 from superset.reports.dao import ReportScheduleDAO
+from superset.reports.models import ReportSchedule
 
 logger = logging.getLogger(__name__)
 

superset/reports/commands/exceptions.py

@@ -24,7 +24,7 @@
     ForbiddenError,
     ValidationError,
 )
-from superset.models.reports import ReportScheduleType
+from superset.reports.models import ReportScheduleType
 
 
 class DatabaseNotFoundValidationError(ValidationError):
@@ -72,7 +72,7 @@ def __init__(self) -> None:
         super().__init__(_("Type is required"), field_name="type")
 
 
-class ReportScheduleChartOrDashboardValidationError(ValidationError):
+class ReportScheduleOnlyChartOrDashboardError(ValidationError):
     """
     Marshmallow validation error for report schedule accept exlusive chart or dashboard
     """
@@ -81,6 +81,17 @@ def __init__(self) -> None:
         super().__init__(_("Choose a chart or dashboard not both"), field_name="chart")
 
 
+class ReportScheduleEitherChartOrDashboardError(ValidationError):
+    """
+    Marshmallow validation error for report schedule missing both dashboard and chart id
+    """
+
+    def __init__(self) -> None:
+        super().__init__(
+            _("Must choose either a chart or a dashboard"), field_name="chart"
+        )
+
+
 class ChartNotSavedValidationError(ValidationError):
     """
     Marshmallow validation error for charts that haven't been saved yet