Skip to content

Commit

Permalink
fix: Fix parsing onSaving reports toast when user hasn't saved chart (#…
Browse files Browse the repository at this point in the history
…16330)

* don't maniuplate error message

* remove extra idx reference

* u

* change print

* update with test

* add case for dashboards

* fix test
  • Loading branch information
hughhhh authored Aug 19, 2021
1 parent 37f09bd commit 50d896f
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 1 deletion.
2 changes: 1 addition & 1 deletion superset-frontend/src/reports/actions/reports.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const addReport = report => dispatch => {
const parsedError = await getClientErrorObject(e);
const errorMessage = parsedError.message;
const errorArr = Object.keys(errorMessage);
const error = errorMessage[errorArr[0]][0];
const error = errorMessage[errorArr[0]];
dispatch(
addDangerToast(
t('An error occurred while editing this report: %s', error),
Expand Down
14 changes: 14 additions & 0 deletions superset/reports/commands/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
from superset.charts.dao import ChartDAO
from superset.commands.base import BaseCommand
from superset.dashboards.dao import DashboardDAO
from superset.models.reports import ReportCreationMethodType
from superset.reports.commands.exceptions import (
ChartNotFoundValidationError,
ChartNotSavedValidationError,
DashboardNotFoundValidationError,
DashboardNotSavedValidationError,
ReportScheduleChartOrDashboardValidationError,
)

Expand All @@ -47,6 +50,17 @@ def validate_chart_dashboard(
""" Validate chart or dashboard relation """
chart_id = self._properties.get("chart")
dashboard_id = self._properties.get("dashboard")
creation_method = self._properties.get("creation_method")

if creation_method == ReportCreationMethodType.CHARTS and not chart_id:
# User has not saved chart yet in Explore view
exceptions.append(ChartNotSavedValidationError())
return

if creation_method == ReportCreationMethodType.DASHBOARDS and not dashboard_id:
exceptions.append(DashboardNotSavedValidationError())
return

if chart_id and dashboard_id:
exceptions.append(ReportScheduleChartOrDashboardValidationError())
if chart_id:
Expand Down
26 changes: 26 additions & 0 deletions superset/reports/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,32 @@ def __init__(self) -> None:
super().__init__(_("Choose a chart or dashboard not both"), field_name="chart")


class ChartNotSavedValidationError(ValidationError):
"""
Marshmallow validation error for charts that haven't been saved yet
"""

def __init__(self) -> None:
super().__init__(
_("Please save your chart first, then try creating a new email report."),
field_name="chart",
)


class DashboardNotSavedValidationError(ValidationError):
"""
Marshmallow validation error for dashboards that haven't been saved yet
"""

def __init__(self) -> None:
super().__init__(
_(
"Please save your dashboard first, then try creating a new email report."
),
field_name="dashboard",
)


class ReportScheduleInvalidError(CommandInvalidError):
message = _("Report Schedule parameters are invalid.")

Expand Down
56 changes: 56 additions & 0 deletions tests/integration_tests/reports/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,62 @@ def test_create_report_schedule_schema(self):
assert data["result"]["timezone"] == "America/Los_Angeles"
assert rv.status_code == 201

@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "create_report_schedules"
)
def test_unsaved_report_schedule_schema(self):
"""
ReportSchedule Api: Test create report schedule with unsaved chart
"""
self.login(username="admin")
chart = db.session.query(Slice).first()
dashboard = db.session.query(Dashboard).first()
example_db = get_example_database()

report_schedule_data = {
"type": ReportScheduleType.REPORT,
"name": "name3",
"description": "description",
"creation_method": ReportCreationMethodType.CHARTS,
"crontab": "0 9 * * *",
"chart": 0,
}
uri = "api/v1/report/"
rv = self.client.post(uri, json=report_schedule_data)
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
assert (
data["message"]["chart"]
== "Please save your chart first, then try creating a new email report."
)

@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "create_report_schedules"
)
def test_no_dashboard_report_schedule_schema(self):
"""
ReportSchedule Api: Test create report schedule with not dashboard id
"""
self.login(username="admin")
chart = db.session.query(Slice).first()
dashboard = db.session.query(Dashboard).first()
example_db = get_example_database()
report_schedule_data = {
"type": ReportScheduleType.REPORT,
"name": "name3",
"description": "description",
"creation_method": ReportCreationMethodType.DASHBOARDS,
"crontab": "0 9 * * *",
}
uri = "api/v1/report/"
rv = self.client.post(uri, json=report_schedule_data)
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
assert (
data["message"]["dashboard"]
== "Please save your dashboard first, then try creating a new email report."
)

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_create_report_schedule_chart_dash_validation(self):
"""
Expand Down

0 comments on commit 50d896f

Please sign in to comment.