feat(api): Add filter_dashboard_id parameter to apply dashboard filters to chart/data endpoint#38638
Conversation
Code Review Agent Run #d74eb2Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
github-actions
Bot
added
api
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| "$ref": "#/components/schemas/ChartDataResponseResult" | ||
| }, | ||
| "type": "array" | ||
| }, |
There was a problem hiding this comment.
So I was not sure the best way to generate the updated OpenAPI docs. I found a command in the Superset CLI, but when I ran that, it added my new changes, but is also deleted a bunch of un-related stuff which made me nervous, so I just went ahead and manually updated the openapi.json file here.
Please let me know if there is a better approach!
There was a problem hiding this comment.
This should do it:
$ superset update_api_docsBut it could be that someone removed or changed some of the APIs without regenerating the spec, and your run is incorporating their changes (which should be fine).
| if dashboard_filter_context.extra_form_data: | ||
| efd = dashboard_filter_context.extra_form_data | ||
| extra_filters = efd.get("filters", []) | ||
|
|
||
| for query in json_body.get("queries", []): | ||
| if extra_filters: | ||
| existing = query.get("filters", []) | ||
| query["filters"] = existing + [ | ||
| {**f, "isExtra": True} for f in extra_filters | ||
| ] | ||
|
|
||
| extras = query.get("extras", {}) | ||
| for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS: | ||
| if key in efd: | ||
| extras[key] = efd[key] | ||
| if extras: | ||
| query["extras"] = extras | ||
|
|
||
| for ( | ||
| src_key, | ||
| target_key, | ||
| ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items(): | ||
| if src_key in efd: | ||
| query[target_key] = efd[src_key] | ||
|
|
||
| query["extra_form_data"] = efd |
There was a problem hiding this comment.
I noticed there are other places in the codebase where we appear to be doing similar things. For example I noticed there is a merge_extra_filters() function utils/core(
superset/superset/utils/core.py
Line 1126 in 6e7d6a8
This is not the best in that we are re-implimenting some logic here that exists in other forms on the front-end/backend, but from my knowledge of the codebase right now I believe this is the best approach. Please let me know if you have thoughts on another approach!
There was a problem hiding this comment.
This also applies to the content in dashboard_filter_context.py. Let me know if you think there is a better approach!
There was a problem hiding this comment.
Yeah, I think this is fine. We're planning to cleanup the query API (#37535) for many reasons, one of them being how complicated and repeated the logic is right now.
| def _get_filter_target_column(filter_config: dict[str, Any]) -> str | None: | ||
| """Extract the target column name from a native filter configuration.""" | ||
| if targets := filter_config.get("targets", []): | ||
| column = targets[0].get("column", {}) | ||
| if isinstance(column, dict): | ||
| return column.get("name") | ||
| if isinstance(column, str): | ||
| return column | ||
| return None |
There was a problem hiding this comment.
I included this target column info to help end users understand how dashboard filters were impacting their queries more precisely, but this might not be essential or super useful depending on the use case for this feature. Can remove if needed
| # We need to apply the form data to the global context as jinga | ||
| # templating pulls form data from the request globally, so this | ||
| # fallback ensures it has the filters and extra_form_data applied | ||
| # when used in get_sqla_query which constructs the final query. | ||
| g.form_data = json_body |
There was a problem hiding this comment.
This was painful to understand lol.
codeant-ai-for-open-source
Bot
added
the
size:XXL
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
codeant-ai-for-open-source
Bot
added
size:XXL
Code Review Agent Run #c1b004Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
codeant-ai-for-open-source
Bot
added
size:XXL
Code Review Agent Run #02340fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
betodealmeida
left a comment
betodealmeida
left a comment
There was a problem hiding this comment.
This is awesome! One question I have, this will still return data along the query, right? Can we pass result_type=query and get just the SQL?
| "$ref": "#/components/schemas/ChartDataResponseResult" | ||
| }, | ||
| "type": "array" | ||
| }, |
There was a problem hiding this comment.
This should do it:
$ superset update_api_docsBut it could be that someone removed or changed some of the APIs without regenerating the spec, and your run is incorporating their changes (which should be fine).
| if dashboard_filter_context.extra_form_data: | ||
| efd = dashboard_filter_context.extra_form_data | ||
| extra_filters = efd.get("filters", []) | ||
|
|
||
| for query in json_body.get("queries", []): | ||
| if extra_filters: | ||
| existing = query.get("filters", []) | ||
| query["filters"] = existing + [ | ||
| {**f, "isExtra": True} for f in extra_filters | ||
| ] | ||
|
|
||
| extras = query.get("extras", {}) | ||
| for key in EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS: | ||
| if key in efd: | ||
| extras[key] = efd[key] | ||
| if extras: | ||
| query["extras"] = extras | ||
|
|
||
| for ( | ||
| src_key, | ||
| target_key, | ||
| ) in EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS.items(): | ||
| if src_key in efd: | ||
| query[target_key] = efd[src_key] | ||
|
|
||
| query["extra_form_data"] = efd |
There was a problem hiding this comment.
Yeah, I think this is fine. We're planning to cleanup the query API (#37535) for many reasons, one of them being how complicated and repeated the logic is right now.
| # We need to apply the form data to the global context as jinga | ||
| # templating pulls form data from the request globally, so this | ||
| # fallback ensures it has the filters and extra_form_data applied | ||
| # when used in get_sqla_query which constructs the final query. | ||
| g.form_data = json_body |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (99.92%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #38638 +/- ##
==========================================
- Coverage 65.54% 64.27% -1.27%
==========================================
Files 1823 2533 +710
Lines 73154 130374 +57220
Branches 23437 30118 +6681
==========================================
+ Hits 47951 83804 +35853
- Misses 25203 45093 +19890
- Partials 0 1477 +1477
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codeant-ai-for-open-source
Bot
added
size:XXL
|
@betodealmeida Yes! I pushed changes to fix the typo and add the re-generated the docs with I am looking at the codecov report and am confused as its showing many tested functions as completely untested, so I will try and figure that out. |
Code Review Agent Run #3fb144Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
codeant-ai-for-open-source
Bot
added
size:XXL
Co-authored-by: codeant-ai-for-open-source[bot] <244253245+codeant-ai-for-open-source[bot]@users.noreply.github.com>
Code Review Agent Run #f4c3d6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
codeant-ai-for-open-source
Bot
added
size:XXL
|
Forgot to comment yesterday: I have shipped one more commit. I unskipped the test with the fixture codeant is claiming to be problematic, as it runs fine locally for me. I will see if it passes in CI and if not that I will re-add the skip. I decided to go with the dict merging approach for handling custom_form_data, as that is what is most aligned with the behaviour elsewhere in the code base. I also saw in the CI run the integration tests failed from a bug with API Spec for the API docs, so I made some changes which should resolve that issue and get CI working. Just updated the branch, I will check in again later today to make sure CI is passing. |
codeant-ai-for-open-source
Bot
added
size:XXL
|
Another note I want to add: Spoke with Beto about this in our last chat, but it appears as if the codecov report here is broken. It is showing that the full contents of the functions within dashboard_filter_context.py are untested when they are. I am not sure if anyone has any insights on how to correct this, but if not this should be safe to ignore codecov |
codeant-ai-for-open-source
Bot
added
size:XXL
…rs to chart/data endpoint (apache#38638) Co-authored-by: Matthew Deadman <matthewdeadman@Matthews-MacBook-Pro-2.local> Co-authored-by: Matthew Deadman <matthewdeadman@matthews-mbp-2.lan> Co-authored-by: codeant-ai-for-open-source[bot] <244253245+codeant-ai-for-open-source[bot]@users.noreply.github.com>
3 participants
User description
SUMMARY
Addresses: #38133
This PR adds a feature to the /api/v1/chart/{id}/data GET endpoint, to enable you to specify an additional parameter: filters_dashboard_id. When you pass in the ID of a dashboard to this parameter, it will apply all the filters on the dashboard which are applicable to this chart.
It will ensure that:
Filters will only be applied to the chart:
The returned result will reflect the applied filters, and include an additional dashboard_filters object, which will show which filters exist on the dashboard, and which were applied to your request. If the user is using the "defaultToFirstItem" it will return a status of "not_applied_uses_default_to_first_item_prequery" to make it very clear to the end user a given filter was not applied because of this option. Column info is also included to help users understand the impact this filter had on their query.
Example:
TESTING INSTRUCTIONS
Spin up superset and create a filter on a dashboard. Make sure it is scoped to hit a chart you want to test. Hit the chart endpoint with filters_dashboard_id = the id of your dashboard with filters. I.E.
GET http://localhost:8088/api/v1/chart/89/data/?filters_dashboard_id=8
Check that query text and results have updated, and that the dashboard_filters include all filters. Remove the scoping from the chart in the filter settings and run again. Make sure the filter has not been applied. Also test removing the default value, and setting "Select first filter value by default" to true and ensuring both show in dashboard filters, but are not applied.
Run the tests:
ADDITIONAL INFORMATION
CodeAnt-AI Description
Apply dashboard filter defaults to chart data requests
What Changed
Impact
✅ Chart data matches dashboard filter defaults✅ Clearer dashboard filter status in API responses✅ Fewer invalid chart data requests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.