Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(explore): datatable crash when dimension is empty #20680

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

Usiel
Copy link
Contributor

@Usiel Usiel commented Jul 12, 2022

Fixes #20679

SUMMARY

react-table wants every column to have a non-empty ID (or Header), else it will crash. So we simply use the current index as a fallback. From what I can see this has no side effects.

Previously fixed with #17303
Reintroduced with #18569

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: https://d.pr/i/KQMrPe
After: https://d.pr/i/2qWjaq

TESTING INSTRUCTIONS

See #20679.

ADDITIONAL INFORMATION

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️
We hope to see you in our Slack community too!

@stephenLYZ
Copy link
Member

Thanks for your contribution! cc @zhaoyongjie @kgabryje

@kgabryje
Copy link
Member

Thanks for the fix! Could you please write a more descriptive comment for this?

@Usiel Usiel force-pushed the usiel/fixes-react-table-error branch from 8a8f764 to 459b313 Compare July 12, 2022 14:09
@Usiel
Copy link
Contributor Author

Usiel commented Jul 12, 2022

Thanks for the fix! Could you please write a more descriptive comment for this?

Oops, of course! Must have fallen asleep before finishing my change 😄

@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #20680 (78b6fd8) into master (8d4994a) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 78b6fd8 differs from pull request most recent head 459b313. Consider uploading reports for the commit 459b313 to get more accurate results

@@            Coverage Diff             @@
##           master   #20680      +/-   ##
==========================================
- Coverage   66.85%   66.85%   -0.01%     
==========================================
  Files        1753     1753              
  Lines       65825    65826       +1     
  Branches     7006     7007       +1     
==========================================
  Hits        44010    44010              
  Misses      20030    20030              
- Partials     1785     1786       +1     
Flag Coverage Δ
javascript 51.95% <0.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
.../src/explore/components/DataTableControl/index.tsx 64.04% <0.00%> (-0.73%) ⬇️

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 8d4994a...459b313. Read the comment docs.

@zhaoyongjie zhaoyongjie merged commit 19247cc into apache:master Jul 13, 2022
lilykuang pushed a commit to preset-io/superset that referenced this pull request Feb 23, 2023
Fixes apache#20679

Co-authored-by: Usiel Riedl <usiel.riedl@automattic.com>
(cherry picked from commit 19247cc)
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 labels Mar 13, 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 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explore Datatable crash when dimension is empty
7 participants