fix(cache): apply dashboard filters to non-legacy visualizations in w…

[richardfogaca]

SUMMARY

Fixes a bug where the /api/v1/dataset/warm_up_cache API endpoint was not applying dashboard filters to non-legacy visualizations

Issue: When warming up cache with a dashboard_id parameter, legacy visualizations correctly applied dashboard default filters, but non-legacy visualizations ignored them completely, caching unfiltered data.

Root cause: In superset/commands/chart/warm_up_cache.py, the non-legacy visualization code path (lines 77-108 in original) did not check for dashboard_id or retrieve dashboard filters, unlike the legacy path which properly called get_dashboard_extra_filters().

Solution:

• Added dashboard filter retrieval and application logic to the non-legacy visualization path
• Refactored code to extract _get_dashboard_filters() helper method (DRY principle)
• Split legacy and non-legacy logic into separate methods to reduce complexity (C901)
• Added 5 comprehensive unit tests to validate filter application

Changes:

1. New method _get_dashboard_filters(): Retrieves filters from extra_filters parameter or dashboard metadata
2. Enhanced _warm_up_non_legacy_cache(): Applies dashboard filters to each query in the query context
3. Refactored run(): Reduced complexity from 11 to 3 by delegating to helper methods

The fix mirrors the existing legacy visualization pattern, ensuring consistent behavior across all visualization types.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend API change only

TESTING INSTRUCTIONS

1. Create a dataset and a chart using a non-legacy visualization (e.g., echarts_timeseries_bar, pie, big_number)
2. Add the chart to a dashboard
3. Configure default filters in the dashboard's json_metadata:

   {"default_filters": "{\"<chart_id>\": {\"<column_name>\": [\"<value>\"]}}"}

4. Call the warm_up_cache API:
   PUT /api/v1/dataset/warm_up_cache
   {
   "db_name": "your_database",
   "table_name": "your_table",
   "dashboard_id": <dashboard_id>
   }
5. Verify the chart data is cached with the dashboard filters applied (check SQL logs or cache keys)

[bito-code-review]

Code Review Agent Run #42cf3a

Actionable Suggestions - 0

Additional Suggestions - 11

• superset/commands/chart/warm_up_cache.py - 4

  • Exception message should use variable assignment · Line 63-63
    Exception message `"Chart's datasource does not exist"` should be assigned to a variable first instead of using string literal directly. This follows best practices for exception handling.
    Code suggestion

     @@ -62,2 +62,3 @@
             if not chart.datasource:
    -            raise ChartInvalidError("Chart's datasource does not exist")
    +            msg = "Chart's datasource does not exist"
    +            raise ChartInvalidError(msg)

  • Exception message should use variable assignment · Line 84-84
    Exception message `"Chart's query context does not exist"` should be assigned to a variable first instead of using string literal directly. This is similar to the issue on line 63.
    Code suggestion

     @@ -83,2 +83,3 @@
             if not query_context:
    -            raise ChartInvalidError("Chart's query context does not exist")
    +            msg = "Chart's query context does not exist"
    +            raise ChartInvalidError(msg)

  • Missing trailing comma in function signature · Line 59-59
    Missing trailing comma after `form_data: dict[str, Any]` in function signature. This improves consistency and makes future parameter additions cleaner.
    Code suggestion

     @@ -58,2 +58,2 @@
         def _warm_up_legacy_cache(
    -        self, chart: Slice, form_data: dict[str, Any]
    +        self, chart: Slice, form_data: dict[str, Any],
         ) -> tuple[Any, Any]:

  • Missing trailing comma in cast expression · Line 90-90
    Missing trailing comma after `dashboard_filters` in the `cast()` function call. This improves consistency with coding style.
    Code suggestion

     @@ -89,2 +89,2 @@
                     query.filter.extend(
    -                    cast(list[QueryObjectFilterClause], dashboard_filters)
    +                    cast(list[QueryObjectFilterClause], dashboard_filters,)
                     )

• tests/unit_tests/commands/chart/warm_up_cache_test.py - 6

  • Nested with statements · Line 51-51
    Combine nested `with` statements into a single statement. Multiple locations (lines 51, 98, 136, 183, 226) have nested `with` statements that can be simplified.
    Code suggestion

     @@ -51,8 +51,6 @@
    -    with patch.object(chart, "get_query_context", return_value=mock_qc):
    -        with patch(
    -            "superset.commands.chart.warm_up_cache.get_form_data",
    -            return_value=[{"viz_type": "echarts_timeseries_bar"}],
    -        ):
    +    with (
    +        patch.object(chart, "get_query_context", return_value=mock_qc),
    +        patch(
    +            "superset.commands.chart.warm_up_cache.get_form_data",
    +            return_value=[{"viz_type": "echarts_timeseries_bar"}],
    +        ),
    +    ):

  • Missing module docstring · Line 1-1
    Add a module-level docstring to describe the purpose of this test file. This helps with code documentation and maintainability.
    Code suggestion

     @@ -17,2 +17,5 @@
    -from unittest.mock import Mock, patch
    -
    +"""Tests for ChartWarmUpCacheCommand functionality."""
    +from unittest.mock import Mock, patch
    +

  • Missing return type annotation · Line 25-25
    Add return type annotations to test functions. Multiple test functions (lines 25, 82, 119, 159, 206) are missing `-> None` return type annotations.
    Code suggestion

     @@ -25,2 +25,2 @@
    -def test_applies_dashboard_filters_to_non_legacy_chart(
    -    mock_chart_data_command, mock_get_dashboard_filters
    +def test_applies_dashboard_filters_to_non_legacy_chart(
    +    mock_chart_data_command, mock_get_dashboard_filters,

  • Missing trailing comma · Line 26-26
    Add trailing commas to function parameter lists. Multiple locations (lines 26, 31, 57, 104, 113, 120, 142, 160, 164, 192, 207, 232) are missing trailing commas.
    Code suggestion

     @@ -26,2 +26,2 @@
    -    mock_chart_data_command, mock_get_dashboard_filters
    -):
    +    mock_chart_data_command, mock_get_dashboard_filters,
    +):

  • Docstring missing ending punctuation · Line 28-28
    Add periods to end docstring sentences. Multiple docstrings (lines 28, 83, 122, 162, 209) are missing ending punctuation.
    Code suggestion

     @@ -28,1 +28,1 @@
    -    """Verify dashboard filters are added to query.filter for non-legacy viz"""
    +    """Verify dashboard filters are added to query.filter for non-legacy viz."""

  • Magic number in comparison · Line 78-78
    Replace magic number `123` with a named constant. This improves code readability and maintainability.
    Code suggestion

     @@ -35,2 +35,4 @@
    -    chart = Slice(
    -        id=123,
    +    CHART_ID = 123
    +    chart = Slice(
    +        id=CHART_ID,

• tests/unit_tests/commands/chart/__init__.py - 1

  • Missing docstring in public package · Line 1-1
    The package `__init__.py` file is missing a docstring. Consider adding a module-level docstring to document the package's purpose and contents.
    Code suggestion

     @@ -16,0 +17,3 @@
      # under the License.
    +"""
    +Unit tests for chart commands.
    +"""

Review Details

• Files reviewed - 3 · Commit Range: fdcc62b..fdcc62b

  • superset/commands/chart/warm_up_cache.py
  • tests/unit_tests/commands/chart/__init__.py
  • tests/unit_tests/commands/chart/warm_up_cache_test.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[korbit-ai]

I've completed my review and didn't find any issues.

Files scanned

​

File Path	Reviewed
superset/commands/chart/warm_up_cache.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[alexandrusoare]

The code looks good overall, just one comment and the CI needs to get fixed

[EnxDev]

Could we add tests for the edge cases?

• Invalid JSON in the extra_filters parameter
• query_context returning None
• Chart without a datasource (legacy path)

[codecov]

Codecov Report

❌ Patch coverage is 71.05263% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.30%. Comparing base (6701d0a) to head (e5a76cb).
⚠️ Report is 40 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/commands/chart/warm_up_cache.py	71.05%	10 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #36109       +/-   ##
===========================================
+ Coverage        0   68.30%   +68.30%     
===========================================
  Files           0      629      +629     
  Lines           0    46198    +46198     
  Branches        0     5000     +5000     
===========================================
+ Hits            0    31554    +31554     
- Misses          0    13395    +13395     
- Partials        0     1249     +1249     

Flag	Coverage Δ
hive	43.90% <15.78%> (?)
mysql	67.41% <71.05%> (?)
postgres	67.46% <71.05%> (?)
presto	47.52% <15.78%> (?)
python	68.26% <71.05%> (?)
sqlite	67.08% <71.05%> (?)
unit	100.00% <ø> (?)

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[richardfogaca]

Comment addressed, CI fixed and tests added. Thanks @alexandrusoare @EnxDev

[EnxDev]

LGTM, thanks!