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

[bugfix] throw "TypeError: 'NoneType'..." when save chart to dash #5835

Merged
merged 2 commits into from
Mar 22, 2019

Conversation

hitdemo
Copy link
Contributor

@hitdemo hitdemo commented Sep 6, 2018

All changes in save_or_overwrite_slice:

  • fix bug
  • change text from "slice" to "chart"
  • make info localizable

Bug replication steps:

  1. remove Admin's "can add on DashboardModelView" permission
  2. open a chart in explore page, then click "save" -> "Add to new dashboard"
  3. a exception is thrown
../incubator-superset/superset/views/core.py", line 1409, in save_or_overwrite_slice
    _('You don\'t have the rights to ') + _('create a ') + _('dashboard'),
TypeError: 'NoneType' object is not callable

it seems that in function save_or_overwrite_slice, lazy_gettext's alies _ is overridden by a local variable _

def save_or_overwrite_slice
        .....
        form_data, _ = self.get_form_data()
        .....

        .....
        _('You don\'t have the rights to ') + _('create a ') + _('dashboard'),
        .....

@kristw kristw added the !deprecated-label:bug Deprecated label - Use #bug instead label Nov 30, 2018
@codecov-io
Copy link

codecov-io commented Mar 21, 2019

Codecov Report

Merging #5835 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5835   +/-   ##
=======================================
  Coverage   64.46%   64.46%           
=======================================
  Files         422      422           
  Lines       20566    20566           
  Branches     2250     2250           
=======================================
  Hits        13257    13257           
  Misses       7182     7182           
  Partials      127      127
Impacted Files Coverage Δ
superset/views/core.py 74.91% <100%> (ø) ⬆️

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

@@ -1437,7 +1437,7 @@ def save_or_overwrite_slice(
"""Save or overwrite a slice"""
slice_name = args.get('slice_name')
action = args.get('action')
form_data, _ = self.get_form_data()
form_data, unused_slc = self.get_form_data()
Copy link
Member

Choose a reason for hiding this comment

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

oh interesting, I never thought that the gettext standard of using _ conflicts with the python standard for unused assignments

@mistercrunch
Copy link
Member

LGTM

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 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants