From e83a9b87a62e8d6b29283b1be08a388940912de7 Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 24 Jan 2022 21:16:21 -0500 Subject: [PATCH 1/6] removed standalone --- superset/reports/commands/execute.py | 2 -- tests/integration_tests/reports/commands_tests.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 7e7eeaceaccd6..2b9920c95ead9 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -72,7 +72,6 @@ DashboardScreenshot, ) from superset.utils.urls import get_url_path -from superset.utils.webdriver import DashboardStandaloneMode logger = logging.getLogger(__name__) @@ -172,7 +171,6 @@ def _get_url( "Superset.dashboard", user_friendly=user_friendly, dashboard_id_or_slug=self._report_schedule.dashboard_id, - standalone=DashboardStandaloneMode.REPORT.value, # force=force, TODO (betodealmeida) implement this **kwargs, ) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index d91d2cd158898..560fa78022c49 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -1767,7 +1767,7 @@ def test_when_tabs_are_selected_it_takes_screenshots_for_every_tabs( assert dashboard_screenshot_mock.call_count == 2 for index, tab in enumerate(tabs): assert dashboard_screenshot_mock.call_args_list[index].args == ( - f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?standalone=3#{tab}", + f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?#{tab}", f"{dashboard.digest}", ) assert send_email_smtp_mock.called is True From 3e75c14c86a4adadc3b5479d35b4d72ce73b152d Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Mon, 24 Jan 2022 22:46:40 -0500 Subject: [PATCH 2/6] Update tests/integration_tests/reports/commands_tests.py --- tests/integration_tests/reports/commands_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 560fa78022c49..8999118f02d51 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -1767,7 +1767,7 @@ def test_when_tabs_are_selected_it_takes_screenshots_for_every_tabs( assert dashboard_screenshot_mock.call_count == 2 for index, tab in enumerate(tabs): assert dashboard_screenshot_mock.call_args_list[index].args == ( - f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?#{tab}", + f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/#{tab}", f"{dashboard.digest}", ) assert send_email_smtp_mock.called is True From ca2b881f2d1e119e099ca9392fbed10a23fb6297 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 25 Jan 2022 12:41:59 -0500 Subject: [PATCH 3/6] changed standalone --- superset/reports/commands/execute.py | 2 ++ tests/integration_tests/reports/commands_tests.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 2b9920c95ead9..7e7eeaceaccd6 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -72,6 +72,7 @@ DashboardScreenshot, ) from superset.utils.urls import get_url_path +from superset.utils.webdriver import DashboardStandaloneMode logger = logging.getLogger(__name__) @@ -171,6 +172,7 @@ def _get_url( "Superset.dashboard", user_friendly=user_friendly, dashboard_id_or_slug=self._report_schedule.dashboard_id, + standalone=DashboardStandaloneMode.REPORT.value, # force=force, TODO (betodealmeida) implement this **kwargs, ) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 560fa78022c49..d91d2cd158898 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -1767,7 +1767,7 @@ def test_when_tabs_are_selected_it_takes_screenshots_for_every_tabs( assert dashboard_screenshot_mock.call_count == 2 for index, tab in enumerate(tabs): assert dashboard_screenshot_mock.call_args_list[index].args == ( - f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?#{tab}", + f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?standalone=3#{tab}", f"{dashboard.digest}", ) assert send_email_smtp_mock.called is True From 565e2c10dd3010a063635d29aeb8b16faf3abf35 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 25 Jan 2022 16:40:50 -0500 Subject: [PATCH 4/6] added tests --- superset/reports/notifications/email.py | 4 ++- superset/utils/urls.py | 11 +++++++- tests/unit_tests/utils/urls_tests.py | 35 +++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 tests/unit_tests/utils/urls_tests.py diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py index 73f30f10636ff..b178e52e5d74e 100644 --- a/superset/reports/notifications/email.py +++ b/superset/reports/notifications/email.py @@ -30,6 +30,7 @@ from superset.reports.notifications.base import BaseNotification from superset.reports.notifications.exceptions import NotificationError from superset.utils.core import send_email_smtp +from superset.utils.urls import get_screenshot_explorelink logger = logging.getLogger(__name__) @@ -94,6 +95,7 @@ def _get_content(self) -> EmailContent: html_table = "" call_to_action = __("Explore in Superset") + url = get_screenshot_explorelink(self._content.url) img_tags = [] for msgid in images.keys(): img_tags.append( @@ -122,7 +124,7 @@ def _get_content(self) -> EmailContent:

{description}

- {call_to_action}

+ {call_to_action}

{html_table} {img_tag} diff --git a/superset/utils/urls.py b/superset/utils/urls.py index 029e2ada7ae76..deead775cad2a 100644 --- a/superset/utils/urls.py +++ b/superset/utils/urls.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import urllib -from typing import Any +from typing import Any, Optional from flask import current_app, url_for @@ -33,3 +33,12 @@ def headless_url(path: str, user_friendly: bool = False) -> str: def get_url_path(view: str, user_friendly: bool = False, **kwargs: Any) -> str: with current_app.test_request_context(): return headless_url(url_for(view, **kwargs), user_friendly=user_friendly) + + +def get_screenshot_explorelink(url: Optional[str]) -> Optional[str]: + data = list(urllib.parse.urlsplit(url)) + params = urllib.parse.parse_qs(data[3]) + params["standalone"] = ["0"] + data[3] = "&".join(f"{k}={urllib.parse.quote(v[0])}" for k, v in params.items()) + url = urllib.parse.urlunsplit(data) + return url diff --git a/tests/unit_tests/utils/urls_tests.py b/tests/unit_tests/utils/urls_tests.py new file mode 100644 index 0000000000000..87a3aba42edcf --- /dev/null +++ b/tests/unit_tests/utils/urls_tests.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +# 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. +from superset.utils.urls import get_screenshot_explorelink + +EXPLORE_CHART_LINK = "http://localhost:9000/superset/explore/?form_data=%7B%22slice_id%22%3A+76%7D&standalone=true&force=false" + +EXPLORE_DASHBOARD_LINK = "http://localhost:9000/superset/dashboard/3/?standalone=3" + + +def test_convert_chart_link() -> None: + test_url = get_screenshot_explorelink(EXPLORE_CHART_LINK) + assert ( + test_url + == "http://localhost:9000/superset/explore/?form_data=%7B%22slice_id%22%3A%2076%7D&standalone=0&force=false" + ) + + +def test_convert_dashboard_link() -> None: + test_url = get_screenshot_explorelink(EXPLORE_DASHBOARD_LINK) + assert test_url == "http://localhost:9000/superset/dashboard/3/?standalone=0" From 89d7e58e935b51f2a5e17d9c2ff4f2e0f298fdaa Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 26 Jan 2022 11:29:17 -0500 Subject: [PATCH 5/6] made function more generic --- superset/reports/notifications/email.py | 6 +++-- superset/utils/urls.py | 22 ++++++++++++------- .../reports/commands_tests.py | 16 +++++++------- tests/unit_tests/utils/urls_tests.py | 6 ++--- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py index b178e52e5d74e..cdec21872c3a4 100644 --- a/superset/reports/notifications/email.py +++ b/superset/reports/notifications/email.py @@ -30,7 +30,7 @@ from superset.reports.notifications.base import BaseNotification from superset.reports.notifications.exceptions import NotificationError from superset.utils.core import send_email_smtp -from superset.utils.urls import get_screenshot_explorelink +from superset.utils.urls import modify_url_query logger = logging.getLogger(__name__) @@ -95,7 +95,9 @@ def _get_content(self) -> EmailContent: html_table = "" call_to_action = __("Explore in Superset") - url = get_screenshot_explorelink(self._content.url) + url = "" + if self._content.url is not None: + url = modify_url_query(self._content.url, standalone="0") img_tags = [] for msgid in images.keys(): img_tags.append( diff --git a/superset/utils/urls.py b/superset/utils/urls.py index deead775cad2a..a8a6148813d96 100644 --- a/superset/utils/urls.py +++ b/superset/utils/urls.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import urllib -from typing import Any, Optional +from typing import Any from flask import current_app, url_for @@ -35,10 +35,16 @@ def get_url_path(view: str, user_friendly: bool = False, **kwargs: Any) -> str: return headless_url(url_for(view, **kwargs), user_friendly=user_friendly) -def get_screenshot_explorelink(url: Optional[str]) -> Optional[str]: - data = list(urllib.parse.urlsplit(url)) - params = urllib.parse.parse_qs(data[3]) - params["standalone"] = ["0"] - data[3] = "&".join(f"{k}={urllib.parse.quote(v[0])}" for k, v in params.items()) - url = urllib.parse.urlunsplit(data) - return url +def modify_url_query(url: str, **kwargs: Any) -> str: + """ + Replace or add parameters to a URL. + """ + parts = list(urllib.parse.urlsplit(url)) + params = urllib.parse.parse_qs(parts[3]) + for k, v in kwargs.items(): + if not isinstance(v, list): + v = [v] + params[k] = v + + parts[3] = "&".join(f"{k}={urllib.parse.quote(v[0])}" for k, v in params.items()) + return urllib.parse.urlunsplit(parts) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index d91d2cd158898..f03666ec9aded 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -694,9 +694,9 @@ def test_email_chart_report_schedule( # assert that the link sent is correct assert ( 'Explore in Superset' + 'standalone=0&force=false">Explore in Superset' in email_mock.call_args[0][2] ) # Assert the email smtp address @@ -737,9 +737,9 @@ def test_email_chart_report_schedule_force_screenshot( # assert that the link sent is correct assert ( 'Explore in Superset' + 'standalone=0&force=true">Explore in Superset' in email_mock.call_args[0][2] ) # Assert the email smtp address @@ -774,9 +774,9 @@ def test_email_chart_alert_schedule( # assert that the link sent is correct assert ( 'Explore in Superset' + 'standalone=0&force=true">Explore in Superset' in email_mock.call_args[0][2] ) # Assert the email smtp address @@ -842,9 +842,9 @@ def test_email_chart_report_schedule_with_csv( # assert that the link sent is correct assert ( 'Explore in Superset' + 'standalone=0&force=false">Explore in Superset' in email_mock.call_args[0][2] ) # Assert the email smtp address diff --git a/tests/unit_tests/utils/urls_tests.py b/tests/unit_tests/utils/urls_tests.py index 87a3aba42edcf..dba38cb81af07 100644 --- a/tests/unit_tests/utils/urls_tests.py +++ b/tests/unit_tests/utils/urls_tests.py @@ -15,7 +15,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from superset.utils.urls import get_screenshot_explorelink +from superset.utils.urls import modify_url_query EXPLORE_CHART_LINK = "http://localhost:9000/superset/explore/?form_data=%7B%22slice_id%22%3A+76%7D&standalone=true&force=false" @@ -23,7 +23,7 @@ def test_convert_chart_link() -> None: - test_url = get_screenshot_explorelink(EXPLORE_CHART_LINK) + test_url = modify_url_query(EXPLORE_CHART_LINK, standalone="0") assert ( test_url == "http://localhost:9000/superset/explore/?form_data=%7B%22slice_id%22%3A%2076%7D&standalone=0&force=false" @@ -31,5 +31,5 @@ def test_convert_chart_link() -> None: def test_convert_dashboard_link() -> None: - test_url = get_screenshot_explorelink(EXPLORE_DASHBOARD_LINK) + test_url = modify_url_query(EXPLORE_DASHBOARD_LINK, standalone="0") assert test_url == "http://localhost:9000/superset/dashboard/3/?standalone=0" From 8c7e2f82669320a48813abc214179e7f2ff1649c Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Wed, 26 Jan 2022 12:32:41 -0500 Subject: [PATCH 6/6] Update superset/reports/notifications/email.py Co-authored-by: Beto Dealmeida --- superset/reports/notifications/email.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py index cdec21872c3a4..da5244a5eac4c 100644 --- a/superset/reports/notifications/email.py +++ b/superset/reports/notifications/email.py @@ -95,9 +95,11 @@ def _get_content(self) -> EmailContent: html_table = "" call_to_action = __("Explore in Superset") - url = "" - if self._content.url is not None: - url = modify_url_query(self._content.url, standalone="0") + 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(