Skip to content

Conversation

@ktmud
Copy link
Member

@ktmud ktmud commented Mar 11, 2020

CATEGORY

  • Bug Fix

SUMMARY

The new table chart implementation added in #9269 and #9234 throws a JS error when query results are empty but the metric list is not empty.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: there is a JS error when query results are empty for a table visualization with metrics:

An error occurred while rendering the visualization: TypeError: Cannot read property '<metric_name>' of undefined

After: it should show "no data available in table".
image

TEST PLAN

  1. Create a new Chart
  2. Add some metrics
  3. Add a query filter that produces empty results
  4. Run the query and check the chart

ADDITIONAL INFORMATION

N/A

REVIEWERS

cc: @etr2460 @kristw @graceguo-supercat

"abbrev": {
"version": "1.1.1",
"resolved": false,
"resolved": "",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the resolved is updated to empty string? Still a fresh npm ci though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@codecov-io
Copy link

Codecov Report

Merging #9275 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9275   +/-   ##
=======================================
  Coverage   58.90%   58.90%           
=======================================
  Files         373      373           
  Lines       12026    12026           
  Branches     2953     2953           
=======================================
  Hits         7084     7084           
  Misses       4763     4763           
  Partials      179      179           

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 724b8a3...042ff9e. Read the comment docs.

Copy link
Member

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

"abbrev": {
"version": "1.1.1",
"resolved": false,
"resolved": "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@etr2460 etr2460 merged commit c9c1801 into apache:master Mar 11, 2020
@ktmud ktmud deleted the bugfix-data-table branch March 11, 2020 06:15
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 First shipped in 0.36.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 0.36.0 First shipped in 0.36.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants