Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChartWarmUpCacheCommand still use legacy filter scopes #29440

Open
3 tasks done
Smallhi opened this issue Jul 1, 2024 · 3 comments
Open
3 tasks done

ChartWarmUpCacheCommand still use legacy filter scopes #29440

Smallhi opened this issue Jul 1, 2024 · 3 comments
Labels
change:backend Requires changing the backend

Comments

@Smallhi
Copy link

Smallhi commented Jul 1, 2024

Bug description

The code show in superset/migrations/shared/native_filters.py:
image

however, superset/views/utils.py, I found the code still use it, which lead to the filter is invalid when warmup.
image

How to reproduce the bug

  1. set filter DEFAULT VALUE
  2. use chart warm up api and warm it.
  3. the DEFAULT VALUE is invalid

Screenshots/recordings

No response

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
@dosubot dosubot bot added the change:backend Requires changing the backend label Jul 1, 2024
Copy link

dosubot bot commented Jul 1, 2024

Hey @Smallhi! 👋 I'm here to help you with any bugs, questions, or contributions you have. Let's squash those bugs together!

To address the issue of ChartWarmUpCacheCommand still using legacy filter scopes, you need to update the superset/views/utils.py file to use native filter configurations. This ensures that the DEFAULT VALUE is valid when using the warmup API.

Here's how you can update the superset/views/utils.py file:

  1. Import the necessary functions and modules:

    from collections import defaultdict
    from typing import Any, List
    from shortid import ShortId
    from superset.utils import json
    from superset.utils.dashboard_filter_scopes_converter import convert_filter_scopes
    from superset.models.slice import Slice
  2. Define the convert_filter_scopes_to_native_filters function:

    def convert_filter_scopes_to_native_filters(
        json_metadata: dict[str, Any],
        position_json: dict[str, Any],
        filter_boxes: List[Slice],
    ) -> List[dict[str, Any]]:
        shortid = ShortId()
        default_filters = json.loads(json_metadata.get("default_filters") or "{}")
        filter_scopes = json_metadata.get("filter_scopes", {})
        filter_box_ids = {filter_box.id for filter_box in filter_boxes}
    
        filter_scope_by_key_and_field: dict[str, dict[str, dict[str, Any]]] = defaultdict(dict)
        filter_by_key_and_field: dict[str, dict[str, dict[str, Any]]] = defaultdict(dict)
    
        for filter_box in filter_boxes:
            key = str(filter_box.id)
            filter_scope_by_key_and_field[key] = {
                **convert_filter_scopes(json_metadata, filter_boxes=[filter_box]).get(filter_box.id, {}),
                **filter_scopes.get(key, {}),
            }
    
        for filter_box in filter_boxes:
            key = str(filter_box.id)
            params = json.loads(filter_box.params or "{}")
    
            for field, filter_scope in filter_scope_by_key_and_field[key].items():
                default = default_filters.get(key, {}).get(field)
    
                fltr: dict[str, Any] = {
                    "cascadeParentIds": [],
                    "id": f"NATIVE_FILTER-{shortid.generate()}",
                    "scope": {
                        "rootPath": filter_scope["scope"],
                        "excluded": [id_ for id_ in filter_scope["immune"] if id_ not in filter_box_ids],
                    },
                    "type": "NATIVE_FILTER",
                }
    
                if field == "__time_col" and params.get("show_sqla_time_column"):
                    fltr.update({
                        "filterType": "filter_timecolumn",
                        "name": "Time Column",
                        "targets": [{"datasetId": filter_box.datasource_id}],
                    })
                    if not default:
                        default = params.get("granularity_sqla")
                    if default:
                        fltr["defaultDataMask"] = {
                            "extraFormData": {"granularity_sqla": default},
                            "filterState": {"value": [default]},
                        }
                elif field == "__time_grain" and params.get("show_sqla_time_granularity"):
                    fltr.update({
                        "filterType": "filter_timegrain",
                        "name": "Time Grain",
                        "targets": [{"datasetId": filter_box.datasource_id}],
                    })
                    if not default:
                        default = params.get("time_grain_sqla")
                    if default:
                        fltr["defaultDataMask"] = {
                            "extraFormData": {"time_grain_sqla": default},
                            "filterState": {"value": [default]},
                        }
                elif field == "__time_range" and params.get("date_filter"):
                    fltr.update({
                        "filterType": "filter_time",
                        "name": "Time Range",
                        "targets": [{}],
                    })
                    if not default:
                        default = params.get("time_range")
                    if default and default != "No filter":
                        fltr["defaultDataMask"] = {
                            "extraFormData": {"time_range": default},
                            "filterState": {"value": default},
                        }
                else:
                    for config in params.get("filter_configs") or []:
                        if config["column"] == field:
                            fltr.update({
                                "controlValues": {
                                    "defaultToFirstItem": False,
                                    "enableEmptyFilter": not config.get("clearable", True),
                                    "inverseSelection": False,
                                    "multiSelect": config.get("multiple", False),
                                    "searchAllOptions": config.get("searchAllOptions", False),
                                },
                                "filterType": "filter_select",
                                "name": config.get("label") or field,
                                "targets": [{"column": {"name": field}, "datasetId": filter_box.datasource_id}],
                            })
                            if "metric" in config:
                                fltr["sortMetric"] = config["metric"]
                                fltr["controlValues"]["sortAscending"] = config["asc"]
                            if params.get("adhoc_filters"):
                                fltr["adhoc_filters"] = params["adhoc_filters"]
                            time_range = params.get("time_range")
                            if time_range and time_range != "No filter":
                                fltr.update({
                                    "time_range": time_range,
                                    "granularity_sqla": params.get("granularity_sqla"),
                                })
                            if not default:
                                default = config.get("defaultValue")
                                if default and config["multiple"]:
                                    default = default.split(";")
                            if default:
                                if not isinstance(default, list):
                                    default = [default]
                                fltr["defaultDataMask"] = {
                                    "extraFormData": {"filters": [{"col": field, "op": "IN", "val": default}]},
                                    "filterState": {"value": default},
                                }
                            break
                if "filterType" in fltr:
                    filter_by_key_and_field[key][field] = fltr
    
        ancestors_by_id = defaultdict(set)
        for filter_box in filter_boxes:
            for value in position_json.values():
                try:
                    if isinstance(value, dict) and value["type"] == "CHART" and value["meta"]["chartId"] == filter_box.id and value["parents"]:
                        ancestors_by_id[filter_box.id] = set(value["parents"])
                except KeyError:
                    pass
    
        for this in filter_boxes:
            for other in filter_boxes:
                if this != other and any(this.id not in filter_scope["immune"] and set(filter_scope["scope"]) <= ancestors_by_id[this.id] for filter_scope in filter_scope_by_key_and_field[str(other.id)].values()):
                    for child in filter_by_key_and_field[str(this.id)].values():
                        if child["filterType"] == "filter_select":
                            for parent in filter_by_key_and_field[str(other.id)].values():
                                if parent["filterType"] in {"filter_select", "filter_time"} and parent["id"] not in child["cascadeParentIds"]:
                                    child["cascadeParentIds"].append(parent["id"])
    
        return sorted([fltr for key in filter_by_key_and_field for fltr in filter_by_key_and_field[key].values()], key=lambda fltr: fltr["filterType"])
  3. Integrate the function into the warmup API:
    Ensure that the warmup API in superset/views/utils.py uses this function to convert legacy filter scopes to native filter configurations.

By following these steps, you can ensure that the ChartWarmUpCacheCommand uses native filter configurations, making the DEFAULT VALUE valid when using the warmup API [1].

To continue talking to Dosu, mention @dosu.

@rusackas
Copy link
Member

rusackas commented Jul 1, 2024

@Smallhi I'm not personally familiar with the ChartWarmUpCacheCommand, so I don't know its intricacies, but it seems like you're close to a PR here :) Would you want to open up a PR/fix? I'm sure @villebro or @michael-s-molina would be happy to review it :D

@Smallhi
Copy link
Author

Smallhi commented Jul 2, 2024

@rusackas ok, I'll fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:backend Requires changing the backend
Projects
None yet
Development

No branches or pull requests

2 participants