fix(excel/csv filename on chart download): downloads zip file and excel/csv file with chart name.

[nandwalritik]

fix(excel/csv filename on chart download): downloads zip file and excel/csv file with chart name.

SUMMARY

When we download any chart as excel/csv file, the file doesn't have appropriate name. So for filename its like

• if the report is paginated it downloads zip folder, whose name is just timestamp for that we can make it like reportName_timstamp.zip.
  • Inside that zip comes 2 files, one which contains the data that can be named as reportname_timestamp.csv and other one reportname_rowcount_timestamp.csv
• In case report is not paginated we can directly name the file as reportname_timestamp.csv.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TESTING INSTRUCTIONS

• Clone the repo, checkout to this branch.
• Run superset > navigate to any dashboard > click on three dots > download excel / download csv > View the filename
• The filename should be same as chart name.

ADDITIONAL INFORMATION

• Has associated issue: Feature Request: Change title of CSV download from timestamp-based to instead be name of chart being exported. #21859
• 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

Hi @sfirke as we discussed over slack,I have made the changes and tested manually on my local.

• For formatting I ran below command

tox -e pre-commit
tox -e pylint

• But while running test cases using pytest as below
  pytest tests/integration_tests/charts/data/api_tests.py
  I am getting below error and unable to find root cause for it
  ERROR tests/integration_tests/charts/data/api_tests.py::test_chart_data_subquery_allowed[200-extras3] - sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such table: dbs

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[sfirke]

thanks for this @nandwalritik, both for taking it so far and for calling out where you need help. I approved the CI tests to run and they failed similar to what you're talking about. I'm not sure what the source for this could be - maybe @john-bodley can take a look during the daily PR review and see if a committer could help get you unstuck?

[nandwalritik]

(sup) nandwalritik@hanoi:~/superset$ tox -e py39 -- tests/integration_tests/charts/data/api_tests.py::TestPostChartDataApi::test_with_multi_query_csv_result_format
py39: commands[0]> superset db upgrade
Traceback (most recent call last):
  File "/home/nandwalritik/superset/.tox/py39/bin/superset", line 33, in <module>
    sys.exit(load_entry_point('apache-superset', 'console_scripts', 'superset')())
  File "/home/nandwalritik/superset/.tox/py39/bin/superset", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/home/nandwalritik/anaconda3/envs/sup/lib/python3.9/importlib/metadata.py", line 86, in load
    module = import_module(match.group('module'))
  File "/home/nandwalritik/anaconda3/envs/sup/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/home/nandwalritik/superset/superset/__init__.py", line 21, in <module>
    from superset.app import create_app
  File "/home/nandwalritik/superset/superset/app.py", line 24, in <module>
    from superset.initialization import SupersetAppInitializer
  File "/home/nandwalritik/superset/superset/initialization/__init__.py", line 35, in <module>
    from superset.extensions import (
  File "/home/nandwalritik/superset/superset/extensions/__init__.py", line 30, in <module>
    from superset.async_events.async_query_manager import AsyncQueryManager
  File "/home/nandwalritik/superset/superset/async_events/async_query_manager.py", line 26, in <module>
    from superset.utils.core import get_user_id
  File "/home/nandwalritik/superset/superset/utils/core.py", line 59, in <module>
    import pandas as pd
  File "/home/nandwalritik/superset/.tox/py39/lib/python3.9/site-packages/pandas/__init__.py", line 22, in <module>
    from pandas.compat import is_numpy_dev as _is_numpy_dev  # pyright: ignore # noqa:F401
  File "/home/nandwalritik/superset/.tox/py39/lib/python3.9/site-packages/pandas/compat/__init__.py", line 29, in <module>
    from pandas.compat.pyarrow import (
  File "/home/nandwalritik/superset/.tox/py39/lib/python3.9/site-packages/pandas/compat/pyarrow.py", line 10, in <module>
    _pa_version = pa.__version__
AttributeError: module 'pyarrow' has no attribute '__version__'
py39: exit 1 (18.95 seconds) /home/nandwalritik/superset> superset db upgrade pid=8878
  py39: FAIL code 1 (19.60=setup[0.65]+cmd[18.95] seconds)
  evaluation failed :( (23.07 seconds)

Hi @john-bodley Can you help me with the test cases part, I am unable to run the Test Cases, once I find a way to run them, I can quickly edit the test cases as well.

[sfirke]

I approved the tests to run on CI at least. As to that local error message, hm, maybe this will help? https://saturncloud.io/blog/solving-the-attributeerror-module-numpy-has-no-attribute-version/

[sfirke]

Closing and reopening to trigger CI

[codecov]

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 83.90%. Comparing base (76d897e) to head (a774d6b).
Report is 859 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/charts/data/api.py	47.05%	9 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #27409       +/-   ##
===========================================
+ Coverage   60.48%   83.90%   +23.41%     
===========================================
  Files        1931      533     -1398     
  Lines       76236    38620    -37616     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    32404    -13710     
+ Misses      28017     6216    -21801     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.95% <11.76%> (-0.22%)	⬇️
javascript	?
mysql	76.71% <47.05%> (?)
postgres	76.84% <47.05%> (?)
presto	53.43% <47.05%> (-0.37%)	⬇️
python	83.90% <47.05%> (+20.41%)	⬆️
sqlite	76.29% <47.05%> (?)
unit	60.91% <11.76%> (+3.29%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nandwalritik]

@sfirke I just added the checks in if condition so the test cases can pass.

[e-belfer]

@nandwalritik and @sfirke Any updates on this PR since July? Is there still a CI issue blocking merge? I'd love to see it merged as it addresses a problem we've been having.

[sfirke]

Restarting CI . Let's pick this back up and try to merge it!

[rusackas]

@gooroodev review

[rusackas]

LGTM... this should replace #25648 and close #25647 when it merges. I'll give the bot a minute to review this, too :)

[sfirke]

@nandwalritik could you please set up pre-commit and get that check passing? This feels so close to mergeable!