fix(explore): Fix chart standalone URL for report/thumbnail generation (#20673)

* Update explore URLs.

* More URL fixes.

* Make frontend accept true/false query params case-insensitively.

* Fix URL mistake.

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
This commit is contained in:
Cody Leff 2022-07-19 12:53:55 -04:00 committed by GitHub
parent e1fd90697c
commit 84d4302628
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 63 additions and 57 deletions

View File

@ -298,7 +298,7 @@ Here's a concrete example:
It's possible to query physical and virtual datasets using the `dataset` macro. This is useful if you've defined computed columns and metrics on your datasets, and want to reuse the definition in adhoc SQL Lab queries.
To use the macro, first you need to find the ID of the dataset. This can be done by going to the view showing all the datasets, hovering over the dataset you're interested in, and looking at its URL. For example, if the URL for a dataset is https://superset.example.org/superset/explore/table/42/ its ID is 42.
To use the macro, first you need to find the ID of the dataset. This can be done by going to the view showing all the datasets, hovering over the dataset you're interested in, and looking at its URL. For example, if the URL for a dataset is https://superset.example.org/explore/?dataset_type=table&dataset_id=42 its ID is 42.
Once you have the ID you can query it as if it were a table:

View File

@ -23,7 +23,7 @@ const MAX_URL_LENGTH = 8000;
export function getURIDirectory(formData, endpointType = 'base') {
// Building the directory part of the URI
let directory = '/superset/explore/';
let directory = '/explore/';
if (['json', 'csv', 'query', 'results', 'samples'].includes(endpointType)) {
directory = '/superset/explore_json/';
}

View File

@ -24,7 +24,7 @@ const MAX_URL_LENGTH = 8000;
export function getURIDirectory(formData, endpointType = 'base') {
// Building the directory part of the URI
let directory = '/superset/explore/';
let directory = '/explore/';
if (['json', 'csv', 'query', 'results', 'samples'].includes(endpointType)) {
directory = '/superset/explore_json/';
}

View File

@ -26,7 +26,7 @@ export const sliceEntitiesForChart = {
slices: {
[sliceId]: {
slice_id: sliceId,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%2018%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%2018%7D',
slice_name: 'Genders',
form_data: {
slice_id: sliceId,
@ -62,7 +62,7 @@ export const sliceEntitiesForDashboard = {
slices: {
[filterId]: {
slice_id: filterId,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20127%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20127%7D',
slice_name: 'Region Filter',
form_data: {
instant_filtering: true,
@ -86,7 +86,7 @@ export const sliceEntitiesForDashboard = {
},
128: {
slice_id: 128,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20128%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20128%7D',
slice_name: "World's Population",
form_data: {},
viz_type: 'big_number',
@ -98,7 +98,7 @@ export const sliceEntitiesForDashboard = {
},
129: {
slice_id: 129,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20129%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20129%7D',
slice_name: 'Most Populated Countries',
form_data: {},
viz_type: 'table',
@ -110,7 +110,7 @@ export const sliceEntitiesForDashboard = {
},
130: {
slice_id: 130,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20130%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20130%7D',
slice_name: 'Growth Rate',
form_data: {},
viz_type: 'line',
@ -122,7 +122,7 @@ export const sliceEntitiesForDashboard = {
},
131: {
slice_id: 131,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20131%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20131%7D',
slice_name: '% Rural',
form_data: {},
viz_type: 'world_map',
@ -134,7 +134,7 @@ export const sliceEntitiesForDashboard = {
},
132: {
slice_id: 132,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20132%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20132%7D',
slice_name: 'Life Expectancy VS Rural %',
form_data: {},
viz_type: 'bubble',
@ -146,7 +146,7 @@ export const sliceEntitiesForDashboard = {
},
133: {
slice_id: 133,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20133%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20133%7D',
slice_name: 'Rural Breakdown',
form_data: {},
viz_type: 'sunburst',
@ -158,7 +158,7 @@ export const sliceEntitiesForDashboard = {
},
134: {
slice_id: 134,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20134%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20134%7D',
slice_name: "World's Pop Growth",
form_data: {},
viz_type: 'area',
@ -170,7 +170,7 @@ export const sliceEntitiesForDashboard = {
},
135: {
slice_id: 135,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20135%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20135%7D',
slice_name: 'Box plot',
form_data: {},
viz_type: 'box_plot',
@ -182,7 +182,7 @@ export const sliceEntitiesForDashboard = {
},
136: {
slice_id: 136,
slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20136%7D',
slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20136%7D',
slice_name: 'Treemap',
form_data: {},
viz_type: 'treemap',

View File

@ -52,10 +52,10 @@ export function getUrlParam({ name, type }: UrlParam): unknown {
if (!urlParam) {
return null;
}
if (urlParam === 'true') {
if (urlParam.toLowerCase() === 'true') {
return 1;
}
if (urlParam === 'false') {
if (urlParam.toLowerCase() === 'false') {
return 0;
}
if (!Number.isNaN(Number(urlParam))) {
@ -71,7 +71,7 @@ export function getUrlParam({ name, type }: UrlParam): unknown {
if (!urlParam) {
return null;
}
return urlParam !== 'false' && urlParam !== '0';
return urlParam.toLowerCase() !== 'false' && urlParam !== '0';
case 'rison':
if (!urlParam) {
return null;

View File

@ -51,7 +51,7 @@ const mockdatasets = [...new Array(3)].map((_, i) => ({
changed_by: 'user',
changed_on: new Date().toISOString(),
database_name: `db ${i}`,
explore_url: `/explore/table/${i}`,
explore_url: `/explore/?dataset_type=table&dataset_id=${i}`,
id: i,
schema: `schema ${i}`,
table_name: `coolest table ${i}`,

View File

@ -535,7 +535,7 @@ class TableModelView( # pylint: disable=too-many-ancestors
resp = super().edit(pk)
if isinstance(resp, str):
return resp
return redirect("/superset/explore/table/{}/".format(pk))
return redirect("/explore/?dataset_type=table&dataset_id={}".format(pk))
@expose("/list/")
@has_access

View File

@ -386,7 +386,7 @@ class Database(
if not source and request and request.referrer:
if "/superset/dashboard/" in request.referrer:
source = utils.QuerySource.DASHBOARD
elif "/superset/explore/" in request.referrer:
elif "/explore/" in request.referrer:
source = utils.QuerySource.CHART
elif "/superset/sqllab/" in request.referrer:
source = utils.QuerySource.SQL_LAB

View File

@ -167,7 +167,7 @@ class BaseReportState:
force=force,
)
return get_url_path(
"Superset.explore",
"ExploreView.root",
user_friendly=user_friendly,
form_data=json.dumps({"slice_id": self._report_schedule.chart_id}),
standalone="true",

View File

@ -20,7 +20,7 @@ Dear {{ user.username }},
<br>
<a href={{ url_for('Superset.profile', username=granter.username, _external=True) }}>
{{ granter.username }}</a> has extended the role {{ role.name }} to include
<a href={{ url_for('Superset.explore', datasource_type=datasource.type, datasource_id=datasource.id, _external=True) }}>
<a href={{ url_for('ExploreView.root', datasource_type=datasource.type, datasource_id=datasource.id, _external=True) }}>
{{datasource.full_name}}</a> and granted you access to it.
<br>
<br>

View File

@ -21,7 +21,7 @@ Dear {{ user.username }},
<a href={{ url_for('Superset.profile', username=granter.username, _external=True) }}>
{{ granter.username }}</a> has granted you the role {{ role.name }}
that gives access to the
<a href={{ url_for('Superset.explore', datasource_type=datasource.type, datasource_id=datasource.id, _external=True) }}>
<a href={{ url_for('ExploreView.root', datasource_type=datasource.type, datasource_id=datasource.id, _external=True) }}>
{{datasource.full_name}}</a>
<br>
<br>

View File

@ -427,13 +427,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
_, slc = get_form_data(slice_id, use_slice_data=True)
if not slc:
abort(404)
endpoint = "/superset/explore/?form_data={}".format(
endpoint = "/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
is_standalone_mode = ReservedUrlParameters.is_standalone_mode()
if is_standalone_mode:
endpoint += f"&{ReservedUrlParameters.STANDALONE}={is_standalone_mode}"
endpoint += f"&{ReservedUrlParameters.STANDALONE}=true"
return redirect(endpoint)
def get_query_string_response(self, viz_obj: BaseViz) -> FlaskResponse:

View File

@ -34,25 +34,40 @@ class R(BaseSupersetView): # pylint: disable=invalid-name
"""used for short urls"""
@staticmethod
def _validate_url(url: Optional[str] = None) -> bool:
if url and (
url.startswith("//superset/dashboard/")
or url.startswith("//superset/explore/")
):
return True
return False
def _validate_explore_url(url: str) -> Optional[str]:
if url.startswith("//superset/explore/p/"):
return url
if url.startswith("//superset/explore"):
return "/" + url[10:] # Remove /superset from old Explore URLs
if url.startswith("//explore"):
return url
return None
@staticmethod
def _validate_dashboard_url(url: str) -> Optional[str]:
if url.startswith("//superset/dashboard/"):
return url
return None
@event_logger.log_this
@expose("/<int:url_id>")
def index(self, url_id: int) -> FlaskResponse:
url = db.session.query(models.Url).get(url_id)
if url and url.url:
explore_url = "//superset/explore/?"
if url.url.startswith(explore_url):
explore_url += f"r={url_id}"
explore_url = self._validate_explore_url(url.url)
if explore_url:
if explore_url.startswith("//explore/?"):
explore_url = f"//explore/?r={url_id}"
return redirect(explore_url[1:])
if self._validate_url(url.url):
return redirect(url.url[1:])
dashboard_url = self._validate_dashboard_url(url.url)
if dashboard_url:
return redirect(dashboard_url[1:])
return redirect("/")
flash("URL to nowhere...", "danger")

View File

@ -123,15 +123,6 @@ class TestCore(SupersetTestCase):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_endpoint(self):
self.login(username="admin")
slc = self.get_slice("Girls", db.session)
resp = self.get_resp("/superset/slice/{}/".format(slc.id))
assert "Original value" in resp
assert "List Roles" in resp
# Testing overrides
resp = self.get_resp("/superset/slice/{}/?standalone=true".format(slc.id))
assert '<div class="navbar' not in resp
resp = self.client.get("/superset/slice/-1/")
assert resp.status_code == 404
@ -881,7 +872,7 @@ class TestCore(SupersetTestCase):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_id_is_always_logged_correctly_on_web_request(self):
# superset/explore case
# explore case
self.login("admin")
slc = db.session.query(Slice).filter_by(slice_name="Girls").one()
qry = db.session.query(models.Log).filter_by(slice_id=slc.id)
@ -1424,7 +1415,7 @@ class TestCore(SupersetTestCase):
"/superset/welcome",
f"/superset/dashboard/{dash_id}/",
"/superset/profile/admin/",
f"/superset/explore/table/{tbl_id}",
f"/explore/?dataset_type=table&dataset_id={tbl_id}",
]
for url in urls:
data = self.get_resp(url)
@ -1566,7 +1557,7 @@ class TestCore(SupersetTestCase):
exception = SupersetException("Error message")
mock_db_connection_mutator.side_effect = exception
slice = db.session.query(Slice).first()
url = f"/superset/explore/?form_data=%7B%22slice_id%22%3A%20{slice.id}%7D"
url = f"/explore/?form_data=%7B%22slice_id%22%3A%20{slice.id}%7D"
self.login()
data = self.get_resp(url)
@ -1576,7 +1567,7 @@ class TestCore(SupersetTestCase):
exception = SQLAlchemyError("Error message")
mock_db_connection_mutator.side_effect = exception
slice = db.session.query(Slice).first()
url = f"/superset/explore/?form_data=%7B%22slice_id%22%3A%20{slice.id}%7D"
url = f"/explore/?form_data=%7B%22slice_id%22%3A%20{slice.id}%7D"
self.login()
data = self.get_resp(url)

View File

@ -724,7 +724,7 @@ def test_email_chart_report_schedule(
)
# assert that the link sent is correct
assert (
'<a href="http://0.0.0.0:8080/superset/explore/?'
'<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>'
@ -769,7 +769,7 @@ def test_email_chart_report_schedule_force_screenshot(
)
# assert that the link sent is correct
assert (
'<a href="http://0.0.0.0:8080/superset/explore/?'
'<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>'
@ -808,7 +808,7 @@ 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/superset/explore/?'
'<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>'
@ -882,7 +882,7 @@ def test_email_chart_report_schedule_with_csv(
)
# assert that the link sent is correct
assert (
'<a href="http://0.0.0.0:8080/superset/explore/?'
'<a href="http://0.0.0.0:8080/explore/?'
"form_data=%7B%22slice_id%22%3A%20"
f"{create_report_email_chart_with_csv.chart.id}%7D&"
'standalone=0&force=false">Explore in Superset</a>'
@ -1260,7 +1260,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/superset/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%20{create_report_slack_chart_with_text.chart.id}%7D&standalone=0&force=false|Explore in Superset>"
in post_message_mock.call_args[1]["text"]
)

View File

@ -17,7 +17,7 @@
# under the License.
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"
EXPLORE_CHART_LINK = "http://localhost:9000/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"
@ -26,7 +26,7 @@ def test_convert_chart_link() -> None:
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"
== "http://localhost:9000/explore/?form_data=%7B%22slice_id%22%3A%2076%7D&standalone=0&force=false"
)