codeant-ai-for-open-source[bot] commented on code in PR #38865:
URL: https://github.com/apache/superset/pull/38865#discussion_r3018730617
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -709,6 +813,78 @@ const FiltersConfigForm = (
() => getAvailableFilters(filterId),
[getAvailableFilters, filterId, filters],
);
+ const chartTitleById = useMemo(
+ () =>
+ chartLayoutItems.reduce<Record<number, string>>((acc, item) => {
+ const chartId = item.meta?.chartId;
+ if (typeof chartId === 'number') {
+ acc[chartId] =
+ item.meta?.sliceNameOverride ||
+ item.meta?.sliceName ||
+ t('Chart %s', chartId);
+ }
+ return acc;
+ }, {}),
+ [chartLayoutItems],
+ );
+ const getNativeFilterChartsInScope = useCallback(
+ (nativeFilterId: string): number[] => {
+ if (removedNativeFilters[nativeFilterId]) {
+ return [];
+ }
+
+ const formNativeFilter = form.getFieldValue([
+ 'filters',
+ nativeFilterId,
+ ]) as NativeFiltersFormItem | undefined;
+ const savedNativeFilter = nativeFilterConfigMap[nativeFilterId];
+ const nativeFilter =
+ formNativeFilter?.type === NativeFilterType.NativeFilter
+ ? formNativeFilter
+ : savedNativeFilter?.type === NativeFilterType.NativeFilter
+ ? savedNativeFilter
+ : undefined;
+
+ if (!nativeFilter) {
+ return [];
+ }
+
+ const scope = formNativeFilter?.scope ?? nativeFilter.scope;
+
+ if (scope && Array.isArray(scope.excluded)) {
+ return getChartIdsInFilterScope(
+ scope,
+ dashboardChartIds,
+ chartLayoutItems,
+ );
+ }
+
+ return 'chartsInScope' in nativeFilter &&
+ Array.isArray(nativeFilter.chartsInScope)
+ ? nativeFilter.chartsInScope
Review Comment:
**Suggestion:** The fallback for native filters without an explicit
scope/charts list currently returns an empty array, which makes a just-created
filter appear to target no charts. That incorrectly excludes valid filters from
dynamic-title token eligibility and validation. Treat missing scope as "all
dashboard charts" to match the default filter behavior used at save time.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dynamic-title controls cannot reference globally scoped filters.
- ⚠️ Template validation may reject otherwise valid filter tokens.
```
</details>
```suggestion
: dashboardChartIds;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard in edit mode and launch the native filter
configuration modal
(component `FiltersConfigModal` in
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx`
which renders `FiltersConfigForm` from `FiltersConfigForm.tsx`).
2. In the modal, create a new native filter and leave its scoping at the
default (do not
open the "Scoping" tab); this results in a filter in form state without an
explicit
`scope` or `chartsInScope` property when read by
`getNativeFilterChartsInScope` in
`FiltersConfigForm.tsx` around lines 831–865.
3. Still in the same modal session, create a new "Chart title template"
display control
(dynamic title) targeting one or more charts; `FiltersConfigForm` (props
`itemType='chartCustomization'`) computes `availableDynamicTitleFilters`
using
`doesNativeFilterCoverCharts`, which calls `getNativeFilterChartsInScope`
(lines 831–865).
Because the new filter has no `scope`/`chartsInScope`, the current
implementation returns
`[]` and `doesNativeFilterCoverCharts(id, currentDynamicTitleChartIds)`
returns `false`.
4. Open the "Insert token" dropdown for the dynamic title (rendered in the
configuration
panel around lines 1645–1665) and observe that the just-created native
filter is not
offered as a token option, even though by default it applies to all
dashboard charts; any
existing template that references such a filter may also be flagged by the
validator
(lines 1670–1749) since `dynamicTitleFilterIds` was built from
`availableDynamicTitleFilters` that excluded it.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
**Line:** 864:864
**Comment:**
*Logic Error: The fallback for native filters without an explicit
scope/charts list currently returns an empty array, which makes a just-created
filter appear to target no charts. That incorrectly excludes valid filters from
dynamic-title token eligibility and validation. Treat missing scope as "all
dashboard charts" to match the default filter behavior used at save time.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38865&comment_hash=a8a803af243f471b404de0473c75db35befcb6f9611f88e5521de002c70c2b3b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38865&comment_hash=a8a803af243f471b404de0473c75db35befcb6f9611f88e5521de002c70c2b3b&reaction=dislike'>👎</a>
--
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]
