feat(reports): adding flag for removing the index on the report CSV

[SkinnyPigeon]

SUMMARY

Some of our account managers send their reports in CSV format. Currently, the index is shown in the CSV due to the POST_PROCESSED value being set in the call to the API. This is causing a few complaints from our clients due to the need for them to remove the column when working with the report data.

There is a current PR that has been ongoing for over a year and appears to have stalled. I have linked this below. Since this is unlikely to be completed, I am opening this in the hopes that we can resolve it sooner. As opposed to their approach, I reformat the url used to call the API based on the CSV_INDEX, leaving it as-is for its default behaviour.

Issue #22981 Current PR

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• [x] Has associated issue:
• [x] 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
• [x] Introduces new feature or API
• [ ] Removes existing feature or API

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.75%. Comparing base (76d897e) to head (8e4dad8). Report is 1502 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/commands/report/execute.py	75.00%	1 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #29335       +/-   ##
===========================================
+ Coverage   60.48%   83.75%   +23.26%     
===========================================
  Files        1931      519     -1412     
  Lines       76236    37637    -38599     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    31524    -14590     
+ Misses      28017     6113    -21904     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.90% <28.57%> (-0.27%)	:arrow_down:
javascript	?
mysql	77.25% <35.71%> (?)
postgres	77.38% <35.71%> (?)
presto	53.50% <28.57%> (-0.31%)	:arrow_down:
python	83.75% <92.85%> (+20.27%)	:arrow_up:
sqlite	76.84% <35.71%> (?)
unit	59.21% <71.42%> (+1.58%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[SkinnyPigeon]

I did wonder about making the remove_post_processing function more general use to allow other mutations to URLs based on flags across the repo. For instance, it could be something like:

def mutate_api_call(url: str, parameter: str) -> str:
    
    Args:
        url (str): The url to process.
        parameter (str): The parameter to remove
    Returns:
        str: The url with the parameter removed."""
    if "?" not in url:
        return url
    base_url, query_string = url.split("?", 1)
    params = query_string.split("&")
    filtered_params = [param for param in params if param != parameter]
    filtered_query_string = "&".join(filtered_params)
    filtered_url = f"{base_url}?{filtered_query_string}"
    return filtered_url

I would then move it into a more general utils location. It would then be called in the same place but changed to:

if not current_app.config["CSV_INDEX"]:
    url = mutate_api_call(url, "type=post_processed")

[john-bodley]

@SkinnyPigeon do you think this would be better handled at the report level as opposed from a system wide configuration?

[SkinnyPigeon]

Yes, you're probably correct 😄. I doubt I would have the capacity to do the changes across both the frontend and backend to achieve that. What are the recommendations for getting a piece of work like that done on this repo?

[michael-s-molina]

@SkinnyPigeon this looks more like a configuration of your report than a global setting.

We should include an option here. @eschutho

[SkinnyPigeon]

Thanks all. I will try to give it a look but if there's anyone you know in the community that's more familiar with working with the modals in the frontend, feel free to pass their names on and I am more than happy to reach out to them and see if we can work together to get this done

[michael-s-molina]

if there's anyone you know in the community that's more familiar with working with the modals in the frontend

I believe @fisjac @rtexelm @dpgaspar recently worked on these modals.

[rusackas]

CC @eschutho @yousoph in case they have any interest in helping to carry this forward. It's so close! :D

[rusackas]

If anyone has interest/bandwidth in picking up the thread on this PR, it would be appreciated, and can close https://github.com/apache/superset/issues/32113