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

[codyml]

SUMMARY

When Explore was moved into the SPA, it changed url from /superset/explore to /explore. This PR updates (hopefully) all of the URL references to /superset/explore that weren't updated accordingly. In detail:

• /superset/explore/?{queryparams} --> /explore/?{queryparams}
• /superset/explore/table/{id} --> /explore/?dataset_type=table&dataset_id={id}
• /superset/explore/p/{hash} --> unchanged
• /superset/explore_json --> unchanged

This should hopefully fix thumbnail and alert generation and other broken links.

The following tweaks are also included:

• The URL that Selenium attempts to access to generate thumbnails is fixed so the standalone query param is true instead of True.
• The frontend now accepts True as a query param for the above, just in case.
• The logic that expands short URLs is updated so previously-created short URLs to Explore should still resolve correctly.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

• Test that thumbnails generate correctly (/testenv up FEATURE_THUMBNAILS=true )
• Test that PNG reports of charts send correctly (/testenv up FEATURE_ALERT_REPORTS=true)
• Test that URLs of the format /superset/slice/{id}/standalone=true resolve correctly to a standalone-mode chart

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[eschutho]

if this is false now, what's the current value? Do we still have a standalone url without the nav?

[codyml]

I believe that now that Explore is in the SPA, the response doc just contains the SPA wrapper. I was assuming that get_resp doesn't download/execute the JS bundle, so I think these tests were only still passing by accident:

• "Original value" and "List Roles" are contained in the embedded bootstrap data but I don't think that bootstrap data is used by Explore anymore (it uses API endpoints now)
• <div class="navbar' isn't present in either standalone or non-standalone mode anymore

So, it seemed like these tests don't test anything meaningful anymore, with the exception of the 404 test below. I looked around for examples of backend tests for Dashboard, but didn't see much and assumed they had also been removed when Dashboard was brought into the SPA.

[ktmud]

Should we make the url parameter case insensitive where it is consumed?

[codecov]

Codecov Report

Merging #20673 (9a7c1c9) into master (c70d102) will decrease coverage by 0.69%.
The diff coverage is 33.22%.

❗ Current head 9a7c1c9 differs from pull request most recent head bc0c8cc. Consider uploading reports for the commit bc0c8cc to get more accurate results

@@            Coverage Diff             @@
##           master   #20673      +/-   ##
==========================================
- Coverage   66.84%   66.14%   -0.70%     
==========================================
  Files        1750     1754       +4     
  Lines       65896    66684     +788     
  Branches     7017     7049      +32     
==========================================
+ Hits        44048    44111      +63     
- Misses      20062    20776     +714     
- Partials     1786     1797      +11     

Flag	Coverage Δ
hive	?
mysql	81.08% <27.71%> (-1.40%)	⬇️
postgres	81.15% <27.71%> (-1.40%)	⬇️
presto	?
python	81.25% <27.71%> (-1.75%)	⬇️
sqlite	80.96% <27.71%> (-1.39%)	⬇️
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ackages/superset-ui-core/src/query/types/Column.ts	100.00% <ø> (ø)
...packages/superset-ui-core/src/query/types/Query.ts	100.00% <ø> (ø)
...ns/legacy-preset-chart-deckgl/src/utils/explore.js	0.00% <0.00%> (ø)
...set-chart-nvd3/src/vendor/superset/exploreUtils.js	0.00% <0.00%> (ø)
...frontend/src/SqlLab/components/SaveQuery/index.tsx	57.89% <ø> (ø)
...rc/SqlLab/components/ScheduleQueryButton/index.tsx	20.75% <0.00%> (ø)
...ntend/src/SqlLab/components/TableElement/index.tsx	72.30% <ø> (ø)
superset-frontend/src/SqlLab/types.ts	57.14% <0.00%> (ø)
superset-frontend/src/components/Chart/Chart.jsx	52.54% <ø> (ø)
...onents/ErrorMessage/ErrorMessageWithStackTrace.tsx	60.00% <ø> (ø)
... and 57 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c70d102...bc0c8cc. Read the comment docs.

[codyml]

Should we make the url parameter case insensitive where it is consumed?

@ktmud I think I'd lean slightly towards keeping it case-sensitive on the client side, just to keep the server-client contract as explicit as possible, but no strong feelings either way!

[ktmud]

In general, restrictions on multi-choice or boolean inputs on the client side should be more relaxed unless it has security implications or significantly increases code complexity. This helps with usability---"you know my intention, just give me what I want already."

[rusackas]

/testenv up

[github-actions]

@rusackas Container image not yet published for this PR. Please try again when build is complete.

[github-actions]

@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.

[zhaoyongjie]

Hi @codyml, I look into the related codebase. I think you just change one line to fix it.

(superset) yongjie.zhao@:incubator-superset$ git diff superset/views/core.py
diff --git a/superset/views/core.py b/superset/views/core.py
index 298535011..8aedf8321 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -427,7 +427,7 @@ 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}))
         )

[michael-s-molina]

@codyml Thank you for the PR. Can we rebase this and check if there's any /superset/explore reference left?

[kgabryje]

Thanks for the fix! The code looks great to me, but I can't fully test it, because for some reason the reports don't work for me on localhost (unrelated to this PR)

[michael-s-molina]

@codyml The license error was fixed here so you just need to rebase your PR.

[zhaoyongjie]

the boolean type can be implicitly converted to string.

Python 3.8.6 (default, Nov 11 2020, 23:52:15)
[Clang 12.0.0 (clang-1200.0.32.21)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> str(True)
'True'
>>> str(False)
'False'
>>>

[codyml]

For this one the implicit conversion to True was part of why the URL wasn't working: the frontend expected it to be lowercase, so I wanted to make that explicit. But, I did update the frontend so if others use implicit conversion from Python bool it should work!

[zhaoyongjie]

I have tested it in my local. the command superset compute-thumbnails -c works and the thumbnail was generated.

[zhaoyongjie]

leave a no-blocking suggestion, others lgtm.

[michael-s-molina]

LGTM

[jinghua-qa]

/testenv up FEATURE_THUMBNAILS=true

[github-actions]

@jinghua-qa Ephemeral environment spinning up at http://34.222.120.222:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[jinghua-qa]

Verified Chart/Dashboard png for Alert and Report is working fine, thumbnail is working for Homepage, Chart and Dashboard
Found 1 issue that deck charts thumbnails are not showing

[jinghua-qa]

@michael-s-molina i am ok with merge this pr first if you think deck is low priority, and we probably need to cherry this in for release candidate.

[michael-s-molina]

@michael-s-molina i am ok with merge this pr first if you think deck is low priority, and we probably need to cherry this in for release candidate.

@jinghua-qa Let's merge this first and follow up with a fix to deck charts.

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.

[sadpandajoe]

🏷️ preset:2022.29