This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 4ddf67fc14 chore: move dashboard screenshot standalone logic (#23003)
4ddf67fc14 is described below
commit 4ddf67fc143f585d00a99510a6a914885dfd9a89
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Wed Feb 15 14:35:08 2023 -0800
chore: move dashboard screenshot standalone logic (#23003)
---
superset/charts/api.py | 4 +--
superset/cli/thumbnails.py | 9 +----
superset/reports/commands/execute.py | 3 --
superset/reports/notifications/email.py | 8 +----
superset/reports/notifications/slack.py | 8 +----
superset/tasks/thumbnails.py | 2 +-
superset/utils/screenshots.py | 20 +++++++++++-
superset/utils/urls.py | 4 ++-
superset/utils/webdriver.py | 5 +++
tests/integration_tests/cli_tests.py | 24 +++++++++++++-
tests/integration_tests/reports/commands_tests.py | 40 +++++++++--------------
tests/unit_tests/utils/urls_tests.py | 5 +++
12 files changed, 76 insertions(+), 56 deletions(-)
diff --git a/superset/charts/api.py b/superset/charts/api.py
index e9ba4feddb..7dc6d5e1e8 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -560,7 +560,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
if not chart:
return self.response_404()
- chart_url = get_url_path("Superset.slice", slice_id=chart.id,
standalone="true")
+ chart_url = get_url_path("Superset.slice", slice_id=chart.id)
screenshot_obj = ChartScreenshot(chart_url, chart.digest)
cache_key = screenshot_obj.cache_key(window_size, thumb_size)
image_url = get_url_path(
@@ -683,7 +683,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
return self.response_404()
current_user = get_current_user()
- url = get_url_path("Superset.slice", slice_id=chart.id,
standalone="true")
+ url = get_url_path("Superset.slice", slice_id=chart.id)
if kwargs["rison"].get("force", False):
logger.info(
"Triggering thumbnail compute (chart id: %s) ASYNC",
str(chart.id)
diff --git a/superset/cli/thumbnails.py b/superset/cli/thumbnails.py
index 5556cff92f..276d9981c1 100755
--- a/superset/cli/thumbnails.py
+++ b/superset/cli/thumbnails.py
@@ -22,7 +22,6 @@ from celery.utils.abstract import CallableTask
from flask.cli import with_appcontext
from superset.extensions import db
-from superset.utils.urls import get_url_path
logger = logging.getLogger(__name__)
@@ -94,13 +93,7 @@ def compute_thumbnails(
action = "Processing"
msg = f'{action} {friendly_type} "{model}" ({i+1}/{count})'
click.secho(msg, fg="green")
- if friendly_type == "chart":
- url = get_url_path(
- "Superset.slice", slice_id=model.id, standalone="true"
- )
- else:
- url = get_url_path("Superset.dashboard",
dashboard_id_or_slug=model.id)
- func(url, model.digest, force=force)
+ func(None, model.id, force=force)
if not charts_only:
compute_generic_thumbnail(
diff --git a/superset/reports/commands/execute.py
b/superset/reports/commands/execute.py
index 8133cec29b..c8edd928b4 100644
--- a/superset/reports/commands/execute.py
+++ b/superset/reports/commands/execute.py
@@ -75,7 +75,6 @@ from superset.utils.core import HeaderDataType, override_user
from superset.utils.csv import get_chart_csv_data, get_chart_dataframe
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
from superset.utils.urls import get_url_path
-from superset.utils.webdriver import DashboardStandaloneMode
logger = logging.getLogger(__name__)
@@ -172,7 +171,6 @@ class BaseReportState:
"ExploreView.root",
user_friendly=user_friendly,
form_data=json.dumps({"slice_id":
self._report_schedule.chart_id}),
- standalone="true",
force=force,
**kwargs,
)
@@ -190,7 +188,6 @@ class BaseReportState:
"Superset.dashboard",
user_friendly=user_friendly,
dashboard_id_or_slug=self._report_schedule.dashboard_id,
- standalone=DashboardStandaloneMode.REPORT.value,
force=force,
**kwargs,
)
diff --git a/superset/reports/notifications/email.py
b/superset/reports/notifications/email.py
index 4b061c8ef6..22b1714f99 100644
--- a/superset/reports/notifications/email.py
+++ b/superset/reports/notifications/email.py
@@ -32,7 +32,6 @@ from superset.reports.notifications.base import
BaseNotification
from superset.reports.notifications.exceptions import NotificationError
from superset.utils.core import HeaderDataType, send_email_smtp
from superset.utils.decorators import statsd_gauge
-from superset.utils.urls import modify_url_query
logger = logging.getLogger(__name__)
@@ -129,11 +128,6 @@ class EmailNotification(BaseNotification): # pylint:
disable=too-few-public-met
html_table = ""
call_to_action = __(app.config["EMAIL_REPORTS_CTA"])
- url = (
- modify_url_query(self._content.url, standalone="0")
- if self._content.url is not None
- else ""
- )
img_tags = []
for msgid in images.keys():
img_tags.append(
@@ -162,7 +156,7 @@ class EmailNotification(BaseNotification): # pylint:
disable=too-few-public-met
<body>
<div>{description}</div>
<br>
- <b><a href="{url}">{call_to_action}</a></b><p></p>
+ <b><a
href="{self._content.url}">{call_to_action}</a></b><p></p>
{html_table}
{img_tag}
</body>
diff --git a/superset/reports/notifications/slack.py
b/superset/reports/notifications/slack.py
index 91470c5b02..b89a700ef9 100644
--- a/superset/reports/notifications/slack.py
+++ b/superset/reports/notifications/slack.py
@@ -44,7 +44,6 @@ from superset.reports.notifications.exceptions import (
NotificationUnprocessableException,
)
from superset.utils.decorators import statsd_gauge
-from superset.utils.urls import modify_url_query
logger = logging.getLogger(__name__)
@@ -63,11 +62,6 @@ class SlackNotification(BaseNotification): # pylint:
disable=too-few-public-met
return json.loads(self._recipient.recipient_config_json)["target"]
def _message_template(self, table: str = "") -> str:
- url = (
- modify_url_query(self._content.url, standalone="0")
- if self._content.url is not None
- else ""
- )
return __(
"""*%(name)s*
@@ -79,7 +73,7 @@ class SlackNotification(BaseNotification): # pylint:
disable=too-few-public-met
""",
name=self._content.name,
description=self._content.description or "",
- url=url,
+ url=self._content.url,
table=table,
)
diff --git a/superset/tasks/thumbnails.py b/superset/tasks/thumbnails.py
index c03d13b0bd..d76939a07e 100644
--- a/superset/tasks/thumbnails.py
+++ b/superset/tasks/thumbnails.py
@@ -48,7 +48,7 @@ def cache_chart_thumbnail(
logger.warning("No cache set, refusing to compute")
return None
chart = cast(Slice, Slice.get(chart_id))
- url = get_url_path("Superset.slice", slice_id=chart.id, standalone="true")
+ url = get_url_path("Superset.slice", slice_id=chart.id)
logger.info("Caching chart: %s", url)
_, username = get_executor(
executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"],
diff --git a/superset/utils/screenshots.py b/superset/utils/screenshots.py
index 741bc9ea5f..a904b7dc43 100644
--- a/superset/utils/screenshots.py
+++ b/superset/utils/screenshots.py
@@ -23,7 +23,13 @@ from typing import Optional, TYPE_CHECKING, Union
from flask import current_app
from superset.utils.hashing import md5_sha_from_dict
-from superset.utils.webdriver import WebDriverProxy, WindowSize
+from superset.utils.urls import modify_url_query
+from superset.utils.webdriver import (
+ ChartStandaloneMode,
+ DashboardStandaloneMode,
+ WebDriverProxy,
+ WindowSize,
+)
logger = logging.getLogger(__name__)
@@ -205,6 +211,11 @@ class ChartScreenshot(BaseScreenshot):
window_size: Optional[WindowSize] = None,
thumb_size: Optional[WindowSize] = None,
):
+ # Chart reports are in standalone="true" mode
+ url = modify_url_query(
+ url,
+ standalone=ChartStandaloneMode.HIDE_NAV.value,
+ )
super().__init__(url, digest)
self.window_size = window_size or (800, 600)
self.thumb_size = thumb_size or (800, 600)
@@ -221,6 +232,13 @@ class DashboardScreenshot(BaseScreenshot):
window_size: Optional[WindowSize] = None,
thumb_size: Optional[WindowSize] = None,
):
+ # per the element above, dashboard screenshots
+ # should always capture in standalone
+ url = modify_url_query(
+ url,
+ standalone=DashboardStandaloneMode.REPORT.value,
+ )
+
super().__init__(url, digest)
self.window_size = window_size or (1600, 1200)
self.thumb_size = thumb_size or (800, 600)
diff --git a/superset/utils/urls.py b/superset/utils/urls.py
index c31bfb1a51..31fbd89337 100644
--- a/superset/utils/urls.py
+++ b/superset/utils/urls.py
@@ -48,7 +48,9 @@ def modify_url_query(url: str, **kwargs: Any) -> str:
v = [v]
params[k] = v
- parts[3] = "&".join(f"{k}={urllib.parse.quote(v[0])}" for k, v in
params.items())
+ parts[3] = "&".join(
+ f"{k}={urllib.parse.quote(str(v[0]))}" for k, v in params.items()
+ )
return urllib.parse.urlunsplit(parts)
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index b1ba784281..e678587029 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -51,6 +51,11 @@ class DashboardStandaloneMode(Enum):
REPORT = 3
+class ChartStandaloneMode(Enum):
+ HIDE_NAV = "true"
+ SHOW_NAV = 0
+
+
def find_unexpected_errors(driver: WebDriver) -> List[str]:
error_messages = []
diff --git a/tests/integration_tests/cli_tests.py
b/tests/integration_tests/cli_tests.py
index f69efc6253..aaa682bee0 100644
--- a/tests/integration_tests/cli_tests.py
+++ b/tests/integration_tests/cli_tests.py
@@ -27,7 +27,9 @@ import yaml
from freezegun import freeze_time
import superset.cli.importexport
-from superset import app
+import superset.cli.thumbnails
+from superset import app, db
+from superset.models.dashboard import Dashboard
from tests.integration_tests.fixtures.birth_names_dashboard import (
load_birth_names_dashboard_with_slices,
load_birth_names_data,
@@ -495,3 +497,23 @@ def test_failing_import_datasets_versioned_export(
)
assert_cli_fails_properly(response, caplog)
+
+
[email protected]("load_birth_names_dashboard_with_slices")
[email protected]("superset.tasks.thumbnails.cache_dashboard_thumbnail")
+def test_compute_thumbnails(thumbnail_mock, app_context, fs):
+
+ thumbnail_mock.return_value = None
+ runner = app.test_cli_runner()
+ dashboard = db.session.query(Dashboard).filter_by(slug="births").first()
+ response = runner.invoke(
+ superset.cli.thumbnails.compute_thumbnails,
+ ["-d", "-i", dashboard.id],
+ )
+
+ thumbnail_mock.assert_called_with(
+ None,
+ dashboard.id,
+ force=False,
+ )
+ assert response.exit_code == 0
diff --git a/tests/integration_tests/reports/commands_tests.py
b/tests/integration_tests/reports/commands_tests.py
index eb606ffec3..8d6a76c14f 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -663,11 +663,9 @@ def test_email_chart_report_schedule(
)
# assert that the link sent is correct
assert (
- '<a href="http://0.0.0.0:8080/explore/?'
- "form_data=%7B%22slice_id%22%3A%20"
- f"{create_report_email_chart.chart.id}%7D&"
- 'standalone=0&force=false">Explore in Superset</a>'
- in email_mock.call_args[0][2]
+ '<a
href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+'
+ f"{create_report_email_chart.chart.id}"
+ '%7D&force=false">Explore in Superset</a>' in
email_mock.call_args[0][2]
)
# Assert the email smtp address
assert email_mock.call_args[0][0] == notification_targets[0]
@@ -720,11 +718,9 @@ def test_email_chart_report_schedule_alpha_owner(
# assert that the link sent is correct
assert (
- '<a href="http://0.0.0.0:8080/explore/?'
- "form_data=%7B%22slice_id%22%3A%20"
- f"{create_report_email_chart_alpha_owner.chart.id}%7D&"
- 'standalone=0&force=false">Explore in Superset</a>'
- in email_mock.call_args[0][2]
+ '<a
href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+'
+ f"{create_report_email_chart_alpha_owner.chart.id}"
+ '%7D&force=false">Explore in Superset</a>' in
email_mock.call_args[0][2]
)
# Assert the email smtp address
assert email_mock.call_args[0][0] == notification_targets[0]
@@ -767,11 +763,9 @@ def test_email_chart_report_schedule_force_screenshot(
)
# assert that the link sent is correct
assert (
- '<a href="http://0.0.0.0:8080/explore/?'
- "form_data=%7B%22slice_id%22%3A%20"
- f"{create_report_email_chart_force_screenshot.chart.id}%7D&"
- 'standalone=0&force=true">Explore in Superset</a>'
- in email_mock.call_args[0][2]
+ '<a
href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+'
+ f"{create_report_email_chart_force_screenshot.chart.id}"
+ '%7D&force=true">Explore in Superset</a>' in
email_mock.call_args[0][2]
)
# Assert the email smtp address
assert email_mock.call_args[0][0] == notification_targets[0]
@@ -806,11 +800,9 @@ def test_email_chart_alert_schedule(
notification_targets =
get_target_from_report_schedule(create_alert_email_chart)
# assert that the link sent is correct
assert (
- '<a href="http://0.0.0.0:8080/explore/?'
- "form_data=%7B%22slice_id%22%3A%20"
- f"{create_alert_email_chart.chart.id}%7D&"
- 'standalone=0&force=true">Explore in Superset</a>'
- in email_mock.call_args[0][2]
+ '<a
href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+'
+ f"{create_alert_email_chart.chart.id}"
+ '%7D&force=true">Explore in Superset</a>' in
email_mock.call_args[0][2]
)
# Assert the email smtp address
assert email_mock.call_args[0][0] == notification_targets[0]
@@ -880,11 +872,9 @@ def test_email_chart_report_schedule_with_csv(
)
# assert that the link sent is correct
assert (
- '<a href="http://0.0.0.0:8080/explore/?'
- "form_data=%7B%22slice_id%22%3A%20"
+ '<a
href="http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+'
f"{create_report_email_chart_with_csv.chart.id}%7D&"
- 'standalone=0&force=false">Explore in Superset</a>'
- in email_mock.call_args[0][2]
+ 'force=false">Explore in Superset</a>' in
email_mock.call_args[0][2]
)
# Assert the email smtp address
assert email_mock.call_args[0][0] == notification_targets[0]
@@ -1313,7 +1303,7 @@ def test_slack_chart_report_schedule_with_text(
| 1 | c21 | c22 | c23 |"""
assert table_markdown in post_message_mock.call_args[1]["text"]
assert (
-
f"<http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A%20{create_report_slack_chart_with_text.chart.id}%7D&standalone=0&force=false|Explore
in Superset>"
+
f"<http://0.0.0.0:8080/explore/?form_data=%7B%22slice_id%22%3A+{create_report_slack_chart_with_text.chart.id}%7D&force=false|Explore
in Superset>"
in post_message_mock.call_args[1]["text"]
)
diff --git a/tests/unit_tests/utils/urls_tests.py
b/tests/unit_tests/utils/urls_tests.py
index a3893953b8..208d6caea4 100644
--- a/tests/unit_tests/utils/urls_tests.py
+++ b/tests/unit_tests/utils/urls_tests.py
@@ -37,6 +37,11 @@ def test_convert_dashboard_link() -> None:
assert test_url ==
"http://localhost:9000/superset/dashboard/3/?standalone=0";
+def test_convert_dashboard_link_with_integer() -> None:
+ test_url = modify_url_query(EXPLORE_DASHBOARD_LINK, standalone=0)
+ assert test_url ==
"http://localhost:9000/superset/dashboard/3/?standalone=0";
+
+
@pytest.mark.parametrize(
"url,is_safe",
[
