fix(charts): Time grain is None when dataset uses Jinja

[Antonio-RiveroMartnez]

SUMMARY

When the underlaying dataset of a chart uses Jinja, some charts are not applying the time_grain properly to the inner query after you change it in the dashboard filters. Regular table charts work just fine.

Looking at what might be causing this, the call we're making to the API is omitting extras.time_grain_sqla when GENERIC_CHART_AXES is enabled which is our default now.

This PR is a first attempt to fix the issue by just putting that back in the API calls so Jinja time_grain is no longer None and gets properly applied.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: time_grain defaults to YEAR ignoring the filter.

After: charts applying the time_grain in the filter.

TESTING INSTRUCTIONS

1. Create a dataset using below SQL query:

with jinja_grain as (
SELECT DATE_TRUNC(
{%- if time_grain == 'P1D' -%}
 'DAY'
{%- elif time_grain == 'P1W' -%}
 'WEEK'
{%- elif time_grain == 'P1M' -%}
 'MONTH'
{%- elif time_grain == 'P3M' -%}
 'QUARTER'
{%- elif time_grain == 'P1Y' -%}
 'YEAR'
{% else %}
 'YEAR'
{%- endif -%}
 ,order_date
) AS order_date_grained, *
FROM "Vehicle Sales" 
)
select * from jinja_grain

2. Use the dataset and create a pivot table. Use order_date_grained for rows and sum(sales) for metrics.
3. Save and add to a dashboard.
4. On the dashboard, add Time Grain filter.
5. Apply a different Time Grain from the filter.

ADDITIONAL INFORMATION

• Has associated issue: time_grain always return None in jinja #22774
• 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

[Antonio-RiveroMartnez]

Hey @zhaoyongjie ! Tagging you here because I'm not very familiar with this part of the codebase, I saw you worked on this before so you might have a better context about whether this approach is correct or what other implications it might have. Thanks

[zhaoyongjie]

@Antonio-RiveroMartnez Thanks for tackling this problem, I totally understood this issue has lots of technical debt, will take a look in the PR.

[betodealmeida]

Looks good! I'm worried that there might be some side effects of including the parameter back, but unfortunately it's very hard to know all the implications of changing this... it would be nice to have a more formalized API for the query object under different conditions.