refactor(explore): simplify form_data parse + add no-datasource regression

/review-code on the post-round-5 slice flagged two minor issues genuinely
introduced by 89919a1c96:

* `loads_request_json` already swallows `TypeError`/`JSONDecodeError` and
  returns `{}`, so the explicit `try/except ValueError` wrapping it was
  dead defensive code. Replace it with an `isinstance(..., dict)` guard
  — that also closes the pre-existing `?form_data=42` edge case where a
  non-dict top-level JSON value would `AttributeError` on `.get()`.

* The no-datasource fall-through path (the loop the fix unblocked) had
  no targeted regression test. `test_slices` and
  `test_slice_id_is_always_logged_correctly_on_web_request` exercised
  it only indirectly via `Slice.slice_url`. Add a sibling to
  `test_explore_redirect` that asserts `/explore/?form_data={"slice_id":1}`
  returns 200 (SPA render), not 302.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joe Li
2026-05-18 20:02:56 -07:00
parent ec7e76eb6c
commit 8a609fd83d
2 changed files with 14 additions and 5 deletions

View File

@@ -44,11 +44,10 @@ class ExploreView(BaseSupersetView):
# so `get_redirect_url` would return the same URL — falling back to
# SPA rendering avoids a 302 loop.
if request_form_data := request.args.get("form_data"):
try:
parsed_form_data = loads_request_json(request_form_data)
except ValueError:
parsed_form_data = {}
if parsed_form_data.get("datasource"):
parsed_form_data = loads_request_json(request_form_data)
if isinstance(parsed_form_data, dict) and parsed_form_data.get(
"datasource"
):
from superset.views.core import Superset # avoid circular import
return redirect(Superset.get_redirect_url())

View File

@@ -882,6 +882,16 @@ class TestCore(SupersetTestCase):
rv = self.client.get(f"/explore/?form_data={quote(json.dumps(form_data))}")
assert rv.headers["Location"] == f"/explore/?form_data_key={random_key}"
def test_explore_no_datasource_renders_spa(self):
# `Slice.slice_url` emits form_data carrying only `slice_id`; without a
# datasource the cache-and-redirect contract can't produce a different
# URL, so ExploreView.root must fall through to the SPA instead of
# 302-looping back to itself.
self.login(ADMIN_USERNAME)
form_data = {"slice_id": 1}
rv = self.client.get(f"/explore/?form_data={quote(json.dumps(form_data))}")
assert rv.status_code == 200
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_has_table(self):
if backend() in ("sqlite", "mysql"):