korbit-ai[bot] commented on code in PR #32196:
URL: https://github.com/apache/superset/pull/32196#discussion_r2004378871
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -1126,6 +1234,79 @@ const AlertReportModal:
FunctionComponent<AlertReportModalProps> = ({
setForceScreenshot(event.target.checked);
};
+ const onChangeDashboardFilter = (idx: number, nativeFilterId: string) => {
+ // set dashboardFilter
+ const anchor = currentAlert?.extra?.dashboard?.anchor;
+
+ // @ts-ignore
+ const inScopeFilters = tabNativeFilters[anchor];
+ const filter = inScopeFilters.filter(
+ (f: any) => f.id === nativeFilterId,
+ )[0];
+
+ const { datasetId } = filter.targets[0];
+ const columnName = filter.targets[0].column.name;
+
+ const columnLabel = nativeFilterOptions.filter(
+ // @ts-ignore
+ filter => filter.value === nativeFilterId,
+ // @ts-ignore
+ )[0].label;
+ const dashboardId = currentAlert?.dashboard?.value;
+
+ // Get values tied to the selected filter
+ const filterValues = {
+ formData: {
+ datasource: `${datasetId}__table`,
+ groupby: [columnName],
+ metrics: ['count'],
+ row_limit: 1000,
+ showSearch: true,
+ viz_type: 'filter_select',
+ type: 'NATIVE_FILTER',
+ dashboardId,
+ },
+ force: false,
+ ownState: {},
+ };
+
+ getChartDataRequest(filterValues).then(response => {
+ const newFilterValues = response.json.result[0].data.map((item: any) =>
({
+ value: item[columnName],
+ label: item[columnName],
+ }));
+
+ console.log('setting filter values', nativeFilterData);
Review Comment:
### Remove debug console.log <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Another debug console.log statement left in production code
###### Why this matters
Debug logs pollute the browser console and may expose sensitive information
in production
###### Suggested change ∙ *Feature Preview*
Remove the console.log statement:
```typescript
// Remove this line
console.log('setting filter values', nativeFilterData);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8253e76-f9c7-4dc9-b5ab-4f4a6272eed8/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8253e76-f9c7-4dc9-b5ab-4f4a6272eed8?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8253e76-f9c7-4dc9-b5ab-4f4a6272eed8?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8253e76-f9c7-4dc9-b5ab-4f4a6272eed8?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8253e76-f9c7-4dc9-b5ab-4f4a6272eed8)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:e5015610-a016-4f52-9c0d-01265376d36a -->
[](e5015610-a016-4f52-9c0d-01265376d36a)
##########
superset/views/core.py:
##########
@@ -846,15 +846,22 @@ def dashboard_permalink(
return redirect("/dashboard/list/")
if not value:
return json_error_response(_("permalink state not found"),
status=404)
+
dashboard_id, state = value["dashboardId"], value.get("state", {})
url = f"/superset/dashboard/{dashboard_id}?permalink_key={key}"
if url_params := state.get("urlParams"):
- params = parse.urlencode(url_params)
- url = f"{url}&{params}"
+ for param_key, param_val in url_params:
+ if param_key == "native_filters":
+ # native_filters doesnt need to be encoded here
+ url = f"{url}&native_filters={param_val}"
Review Comment:
### Unclear native_filters special case documentation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The comment explaining native_filters behavior lacks clarity on why encoding
is not needed.
###### Why this matters
Without understanding why native_filters is treated differently, future
developers might accidentally change this behavior and introduce bugs.
###### Suggested change ∙ *Feature Preview*
# native_filters is a JSON string that's already properly encoded
# and requires special handling to preserve its structure
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/735ad139-b8a2-43c8-bea2-5ce4998fc1e1/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/735ad139-b8a2-43c8-bea2-5ce4998fc1e1?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/735ad139-b8a2-43c8-bea2-5ce4998fc1e1?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/735ad139-b8a2-43c8-bea2-5ce4998fc1e1?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/735ad139-b8a2-43c8-bea2-5ce4998fc1e1)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6d4efbb4-b138-4a29-9868-7b3221c9bf8b -->
[](6d4efbb4-b138-4a29-9868-7b3221c9bf8b)
##########
superset/dashboards/api.py:
##########
@@ -471,7 +471,11 @@ def get_tabs(self, id_or_slug: str) -> Response:
""" # noqa: E501
try:
tabs = DashboardDAO.get_tabs_for_dashboard(id_or_slug)
+ native_filters =
DashboardDAO.get_native_filter_configuration(id_or_slug)
Review Comment:
### Inefficient Sequential Database Queries <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Sequential database queries are being made to fetch tabs and native filters
separately, which could be combined into a single database operation.
###### Why this matters
Multiple sequential database round-trips increase latency and database load.
This is especially important in API endpoints where response time is critical.
###### Suggested change ∙ *Feature Preview*
Modify DashboardDAO to provide a single method that fetches both tabs and
native filters in one query:
```python
@classmethod
def get_tabs_and_filters(cls, id_or_slug: str) -> Tuple[Dict, Dict]:
# Combine the queries using appropriate joins
return tabs, native_filters
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ccc050d-f980-408b-8aa9-ecae7fd3d9d6/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ccc050d-f980-408b-8aa9-ecae7fd3d9d6?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ccc050d-f980-408b-8aa9-ecae7fd3d9d6?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ccc050d-f980-408b-8aa9-ecae7fd3d9d6?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ccc050d-f980-408b-8aa9-ecae7fd3d9d6)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:52cb7fc0-980a-47b2-97ee-ff9dd99e2a79 -->
[](52cb7fc0-980a-47b2-97ee-ff9dd99e2a79)
##########
superset/config.py:
##########
@@ -494,7 +494,7 @@ class D3TimeFormat(TypedDict, total=False):
"EMBEDDED_SUPERSET": False,
# Enables Alerts and reports new implementation
"ALERT_REPORTS": False,
- "ALERT_REPORT_TABS": False,
+ "ALERT_REPORT_TABS": True,
Review Comment:
### Alert Reports Feature Flag Dependency Mismatch <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The ALERT_REPORT_TABS flag is enabled but its dependent feature flag
ALERT_REPORTS is still disabled (set to False). This prevents the native
filters functionality from working properly since the main feature needs to be
enabled first.
###### Why this matters
Having ALERT_REPORT_TABS enabled without ALERT_REPORTS will result in the
new native filters UI being shown but the underlying functionality not working,
leading to a broken user experience.
###### Suggested change ∙ *Feature Preview*
Enable both required feature flags to ensure proper functionality:
```python
"ALERT_REPORTS": True,
"ALERT_REPORT_TABS": True,
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/825141e4-a44d-45c0-b545-49d59f3d9ecd/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/825141e4-a44d-45c0-b545-49d59f3d9ecd?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/825141e4-a44d-45c0-b545-49d59f3d9ecd?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/825141e4-a44d-45c0-b545-49d59f3d9ecd?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/825141e4-a44d-45c0-b545-49d59f3d9ecd)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:04e71b7a-bd82-47ed-a102-e0de1f1bbc6a -->
[](04e71b7a-bd82-47ed-a102-e0de1f1bbc6a)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
