Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix parsing onSaving reports toast when user hasn't saved chart #16330

Merged
merged 7 commits into from
Aug 19, 2021
Merged

fix: Fix parsing onSaving reports toast when user hasn't saved chart #16330

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
40dbdb3
don't maniuplate error message
hughhhh Aug 18, 2021
f8d9614
remove extra idx reference
hughhhh Aug 18, 2021
9d5fd18
u
hughhhh Aug 18, 2021
627079d
change print
hughhhh Aug 18, 2021
9a659d7
update with test
hughhhh Aug 19, 2021
82fe7c5
add case for dashboards
hughhhh Aug 19, 2021
ae5de18
fix test
hughhhh Aug 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/src/reports/actions/reports.js
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion superset/reports/commands/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from superset.dashboards.dao import DashboardDAO
from superset.reports.commands.exceptions import (
ChartNotFoundValidationError,
ChartNotSavedValidationError,
DashboardNotFoundValidationError,
ReportScheduleChartOrDashboardValidationError,
)
Expand Down Expand Up @@ -60,4 +61,4 @@ def validate_chart_dashboard(
exceptions.append(DashboardNotFoundValidationError())
self._properties["dashboard"] = dashboard
elif not update:
exceptions.append(ReportScheduleChartOrDashboardValidationError())
exceptions.append(ChartNotSavedValidationError())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible that this could be a dashboard?

Copy link
Member Author

@hughhhh hughhhh Aug 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding dashboards would always be saved and have a record in the metastore, but for a more defensive approach i'll make a specific condition for both dashboards and charts just in case.

12 changes: 12 additions & 0 deletions superset/reports/commands/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ 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 ReportScheduleInvalidError(CommandInvalidError):
message = _("Report Schedule parameters are invalid.")

Expand Down