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

[forms] Fix handling of NULLs for chart or dashboard name #7078

Conversation

john-bodley
Copy link
Member

@john-bodley john-bodley commented Mar 21, 2019

This PR resolves a couple of issues related to either a chart or dashboard name being None. Prior to #5445 these fields were encoded as empty strings and thus certain issues didn't surface.

Since #5445 it is impossible to filter by either dashboard or chart in which the name (title) is undefined as there is no mechanism to filter either in/out NULL values. Furthermore sorting by either chart of dashboard name isn't viable due to an issue with Flask-AppBuilder as mentioned in #3673. On the flip side when empty strings were allowed it was possible to filter these entities however itwas not possible to navigate to said entity from the CRUD view as there was no link present.

This PR additionally renames undefined names to <empty> in the CRUD view as this is consistent with both the Dashboard and Explorer views.

Charts
Screen Shot 2019-03-21 at 12 15 42 AM
Screen Shot 2019-03-21 at 12 15 55 AM

Dashboards
Screen Shot 2019-03-21 at 12 16 04 AM
Screen Shot 2019-03-21 at 12 16 19 AM

to: @graceguo-supercat @michellethomas @mistercrunch @xtinec

@codecov-io
Copy link

codecov-io commented Mar 21, 2019

Codecov Report

Merging #7078 into master will increase coverage by <.01%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7078      +/-   ##
==========================================
+ Coverage   64.46%   64.46%   +<.01%     
==========================================
  Files         422      422              
  Lines       20566    20570       +4     
  Branches     2250     2250              
==========================================
+ Hits        13257    13260       +3     
- Misses       7182     7183       +1     
  Partials      127      127
Impacted Files Coverage Δ
superset/views/core.py 74.91% <100%> (ø) ⬆️
superset/models/core.py 83.56% <75%> (-0.06%) ⬇️

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 b210742...c231be6. Read the comment docs.

@john-bodley john-bodley force-pushed the john-bodley--untitled-charts-and-dashboards branch from c231be6 to 8aa65e6 Compare March 21, 2019 18:32
@john-bodley john-bodley added v0.31 !deprecated-label:bug Deprecated label - Use #bug instead and removed v0.31 labels Mar 22, 2019
Copy link

@graceguo-supercat graceguo-supercat left a comment

Choose a reason for hiding this comment

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

LGTM

@john-bodley john-bodley merged commit b3c4bd9 into apache:master Mar 25, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.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 !deprecated-label:bug Deprecated label - Use #bug instead v0.31 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants